svn commit: r333351 - head/usr.bin/grep
Ronald Klop
ronald-lists at klop.ws
Tue May 8 20:36:55 UTC 2018
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?
Ronald.
More information about the svn-src-head
mailing list