Max Horn <m...@quendi.de> writes:

>> +            buf = ident_line;
>>              if (split_ident_line(&ident,
>> -                                 buf + strlen("author "),
>> -                                 line_end - (buf + strlen("author "))) ||
>> +                                 buf,
>> +                                 line_end - buf) ||
>>                  !ident.date_begin || !ident.date_end)
>>                      goto fail_exit; /* malformed "author" line */
>>              break;
>
> Why not get rid of that assignment to "buf", and use ident_line
> instead of buf below? That seems like it would be more readable,
> wouldn't it?

Yes, and also now the argument list is much shorter, you could
probably do it on two lines instead of three:

                if (split_ident_line(&ident,
                                     ident_line, line_end - ident_line) ||
                    ...


>> @@ -1193,10 +1195,9 @@ static void parse_gpg_output(struct signature_check 
>> *sigc)
>>      for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
>>              const char *found, *next;
>> 
>> -            if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
>> -                    /* At the very beginning of the buffer */
>> -                    found = buf + strlen(sigcheck_gpg_status[i].check + 1);
>> -            } else {
>> +            found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
>> +            /* At the very beginning of the buffer */
>
> Do we really need that comment, and in that spot? The code seemed
> clear enough to me without it. But if you think keeping is better,
> perhaps move it to *before* the skip_prefix, and add a trailing
> "?"

Both good suggestions (I tend to prefer the removal).

Thanks.
--
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