Re: [PATCH] builtin-tag.c: remove global variable to use the callback data of git-config.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 17, 2009 at 11:27 PM, Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
> Hi,
>
> On Tue, 17 Mar 2009, Carlos Rica wrote:
>
>> On Tue, Mar 17, 2009 at 4:47 PM, Johannes Schindelin
>> <Johannes.Schindelin@xxxxxx> wrote:
>> > Hi,
>> > On Tue, 17 Mar 2009, Carlos Rica wrote:
>> >> @@ -164,11 +162,10 @@ static int do_sign(struct strbuf *buffer)
>> >>       int len;
>> >>       int i, j;
>> >>
>> >> -     if (!*signingkey) {
>> >> -             if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
>> >> -                             sizeof(signingkey)) > sizeof(signingkey) - 1)
>> >> -                     return error("committer info too long.");
>> >> -             bracket = strchr(signingkey, '>');
>> >> +     if (!signingkey->buf[0]) {
>> >
>> > It is probably better to ask for !signingkey->len (think of trying to
>> > understand the code in 6 months from now).
>>
>> I was in doubt here. By avoiding the use of signingkey->len  I was
>> trying to say that you cannot rely in such field if we touch the
>> buffer directly, as it happens below:
>>
>>    bracket = strchr(signingkey->buf, '>');
>>    if (bracket)
>>       bracket[1] = '\0';
>
> Oh, I missed that.  It should read
>
>        if (bracket)
>                strbuf_setlen(signingkey, bracket + 1 - signingkey->buf);
>
> instead.

I agree! That's much better. Thanks Dscho and Junio.
Tell me if you need me to resend the patch with both changes.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux