On Tue, 08 May 2018 16:49:12 +0200, Alexey Dokuchaev <da...@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 <da...@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.
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to