blacksimit <cengoguzhanu...@gmail.com> writes:

> From: Oguzhan Unlu <cengoguzhanu...@gmail.com>
>
> My solution to make lines containing buffer += a_number; clearer to anyone is 
> following; I defined a new int, magic_num, then assigned lengths of used 
> strings to magic_num and then changed assignment lines through using 
> magic_num so that where the number which is added to buffer is known although 
> I eliminated 3rd parameter of memcmp() when using starts_with().   
Eric told you in $gmane/244637:

> Wrap messages to 65-70 characters.

I do not think replacing 5 (or 7) with a variable makes anything
clearer; in fact, by forcing people to check what the last value
assigned to the variable every time they see "+magic_num", the
resulting code is much harder to read, I would think.

Among the various submissions, I found this one one of the cleaner
solutions:

    http://thread.gmane.org/gmane.comp.version-control.git/244019/focus=244020

and it has been queued as 2d820a61 (fsck.c:fsck_commit(): use
skip_prefix() to verify and skip constant, 2014-03-13) on 'pu'.

> Signed-off-by: Oguzhan Unlu <cengoguzhanu...@gmail.com>
> ---
>  I didn't use skip_prefix() in this microproject and I plan to apply for GSOC 
> 2014. I expect your feedbacks!
>  fsck.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/fsck.c b/fsck.c
> index d43be98..4e5ca30 100644
> --- a/fsck.c
> +++ b/fsck.c
> @@ -289,14 +289,17 @@ static int fsck_commit(struct commit *commit, 
> fsck_error error_func)
>       struct commit_graft *graft;
>       int parents = 0;
>       int err;
> -
> +    int magic_num;
> +    
> +    magic_num = strlen("tree "); /* magic_num is 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')
> +     if (get_sha1_hex(buffer+magic_num, tree_sha1) || buffer[45] != '\n')
>               return error_func(&commit->object, FSCK_ERROR, "invalid 'tree' 
> line format - bad sha1");
>       buffer += 46;
> +    magic_num = strlen("parent "); /* magic_num is 7 */
>       while (starts_with(buffer, "parent ")) {
> -             if (get_sha1_hex(buffer+7, sha1) || buffer[47] != '\n')
> +             if (get_sha1_hex(buffer+magic_num, sha1) || buffer[47] != '\n')
>                       return error_func(&commit->object, FSCK_ERROR, "invalid 
> 'parent' line format - bad sha1");
>               buffer += 48;
>               parents++;
> @@ -322,15 +325,17 @@ static int fsck_commit(struct commit *commit, 
> fsck_error error_func)
>               if (p || parents)
>                       return error_func(&commit->object, FSCK_ERROR, "parent 
> objects missing");
>       }
> +    magic_num = strlen("author "); /* magic_num is 7 */
>       if (!starts_with(buffer, "author "))
>               return error_func(&commit->object, FSCK_ERROR, "invalid format 
> - expected 'author' line");
> -     buffer += 7;
> +     buffer += magic_num;
>       err = fsck_ident(&buffer, &commit->object, error_func);
>       if (err)
>               return err;
> +    magic_num = strlen("committer"); /* magic_num is 7 */
>       if (!starts_with(buffer, "committer "))
>               return error_func(&commit->object, FSCK_ERROR, "invalid format 
> - expected 'committer' line");
> -     buffer += strlen("committer ");
> +     buffer += magic_num;
>       err = fsck_ident(&buffer, &commit->object, error_func);
>       if (err)
>               return err;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to