Re: [PATCH/RFC 1/2] fetch-pack: new option to read refs from stdin

From: Ivan Todoroski <grnch_lists@gmx.net>
Date: Mar 25, 2012, 15:15 UTC
Message-ID: <4F6F3683.7000900@gmx.net>
In-Reply-To: <4F6EE7E2.1020702@gmx.net>

On 25.03.2012 11:39, Ivan Todoroski wrote:

Show 21 quoted lines

> On 25.03.2012 03:19, Jeff King wrote:
>> On Sat, Mar 24, 2012 at 09:53:26PM +0100, Ivan Todoroski wrote:
>>> @@ -972,6 +976,42 @@ int cmd_fetch_pack(int argc, const char **argv, 
>>> const char *prefix)
>>>      if (!dest)
>>>          usage(fetch_pack_usage);
>>>  
>>> +    if (args.refs_from_stdin) {
>>> +        char ref[1000];
>>
>> Ick. Is there any reason not to use a strbuf here? 1000 is probably
>> plenty, but we are generally moving towards removing such limits where
>> possible.
>>
>> You'd also get to use strbuf_getline and strbuf_trim in the
>> newline-delimited case.
> 
> Right now that "char ref[1000]" code is rejecting refs on stdin if they 
> are longer than 1000 chars or if they contain an ASCII NUL char in them. 
> When I change this to strbuf_getline() should I be doing any similar 
> checks, or do I just pass on whatever I read?

Never mind, there is no checking necessary. I just saw that strbuf_getline() grows the buffer automatically so there is no chance of buffer overflows, plus it doesn't mind if NUL chars are present in the middle of the string. The subsequent code that reads the refs will stop at the first NUL char anyway so all is well.

I didn't know about this cool strbuf infrastructure, thanks for the tip.