svn commit: r333351 - head/usr.bin/grep

[Kyle Evans]

On Tue, May 8, 2018 at 3:36 PM, Ronald Klop <ronald-lists at klop.ws> wrote:
> On Tue, 08 May 2018 16:49:12 +0200, Alexey Dokuchaev <danfe at freebsd.org>
> wrote:
>
>> On Tue, May 08, 2018 at 09:36:21AM -0500, Kyle Evans wrote:
>>>
>>> On Tue, May 8, 2018 at 5:58 AM, Alexey Dokuchaev <danfe at freebsd.org>
>>> wrote:
>>> >>
>>> >> -     if ((f = fopen(fn, "r")) == NULL)
>>> >> +     if (strcmp(fn, "-") == 0)
>>> >> +             f = stdin;
>>> >
>>> > This makes sense: when `fn' is "-", `f' is stdin.
>>> >
>>> >> -     fclose(f);
>>> >> +     if (strcmp(fn, "-") != 0)
>>> >> +             fclose(f);
>>> >
>>> > But not this one: why are you checking `fn' again?  Shouldn't you
>>> > fclose(f) if it's not stdin?
>>> >
>>> >         if (f != stdin)
>>> >                 fclose(f);
>>> >
>>>
>>> You say potato, I say potato. =) In this case, it's low overhead in a
>>> not particularly performance critical bit and drawing a connection
>>> between this and the opening of 'f' above in an extremely obvious way.
>>
>>
>> Well, I'm not worried about the overhead or performance issues, they are
>> negligible.  I just find second strcmp(fn, "-") to be semantically wrong
>> (and that's why you need implicit "there's only one way to get stdin here"
>> assert).  You assign `f' to stdin based on `fn' being "-", but you
>> fclose(f) when it's not stdin; the value of `fn' is irrelevant this time.
>> As a nice bonus, you only spell strcmp(fn, "-") once and do not need to
>> implicitly assert that there's only one way to get stdin here.
>>
>>> This also might get ripped out soon -- we'll see how things go.
>>
>>
>> I see, understood.
>>
>> ./danfe
>
>
> What is the result of "-f /dev/stdin"? Of does that fopen return a different
> filedesc?
>

Indeed- /dev/stdin will force it to open another filedesc and read
from stdin all the same.

Thanks,

Kyle Evans