git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] fsck.c:fsck_commit() use starts_with() and skip_prefix()

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Mar 18, 2014, 23:16 UTC
Message-ID
<5328D3C0.5080009@alum.mit.edu>
In-Reply-To
<CAPig+cRoRzZKjW3cY86iQxz9iZ_TKT-yyu=6Va5aV_DsCAQquA@mail.gmail.com>
On 03/19/2014 12:09 AM, Eric Sunshine wrote:
Show 41 quoted lines
> Thanks for the submission. Comments below to give you a taste of the
> Git review process...
> 
> On Tue, Mar 18, 2014 at 6:41 PM, Othman Darraz <darrazo16@gmail.com> wrote:
>> use of starts_with() instead of memcmp()
>>
>> use of skip_prefix instead of memcmp() and strlen()
> 
> Write proper sentences to explain and justify the change; not sentence
> fragments.
> 
>> Signed-off-by: Othman Darraz <darrazo16@gmail.com>
>> ---
>>
>> I am planning to apply to GSOC 214
> 
> Your logic in these changes is rather severely flawed. Running the
> test suite (t1450, in particular), as the GSoC microproject page
> suggests, would have clued you in that something was amiss.
> 
>>  fsck.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fsck.c b/fsck.c
>> index 64bf279..5eae856 100644
>> --- a/fsck.c
>> +++ b/fsck.c
>> @@ -290,7 +290,7 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)
>>         int parents = 0;
>>         int err;
>>
>> -       if (memcmp(buffer, "tree ", 5))
>> +       if (starts_with(buffer, "tree "))
>>                 return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line");
>>         if (get_sha1_hex(buffer+5, tree_sha1) || buffer[45] != '\n')
> 
> One of the reasons for using starts_with() rather than memcmp() is
> that it allows you to eliminate magic numbers, such as 5. However, if
> you look closely at this code fragment, you will see that the magic
> number is still present in the expression 'buffer+5'. starts_with(),
> might be a better fit.
Eric, I think you meant to say that *skip_prefix()* might be a better fit.
> [...]
Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Previous: Eric SunshineNext: Eric Sunshine
Message 4 of 5 in “fsck.c:fsck_commit() use starts_with() and skip_prefix()”
  1. fsck.c:fsck_commit() use starts_with() and skip_prefix()Othman Darraz, Mar 18, 2014
  2. Eric SunshineMar 18, 2014
  3. Eric SunshineMar 18, 2014
  4. Michael HaggertyMar 18, 2014
  5. Eric SunshineMar 19, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.