Date: Tue, 08 May 2018 22:36:50 +0200 From: "Ronald Klop" <ronald-lists@klop.ws> To: "Kyle Evans" <kevans@freebsd.org>, "Alexey Dokuchaev" <danfe@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers <src-committers@freebsd.org> Subject: Re: svn commit: r333351 - head/usr.bin/grep Message-ID: <op.zipp7obvkndu52@klop.ws> In-Reply-To: <20180508144912.GA3581@FreeBSD.org> References: <201805080353.w483rlde033542@repo.freebsd.org> <20180508105815.GB7299@FreeBSD.org> <CACNAnaEiV7kC%2BUNy6GwiEtLzOqwA%2BkPuunDNBtb1dQUQeb3uzQ@mail.gmail.com> <20180508144912.GA3581@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 08 May 2018 16:49:12 +0200, Alexey Dokuchaev <danfe@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@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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?op.zipp7obvkndu52>