[Toybox] [PATCH 1/2] tac: switch to getline().

[enh]

On Wed, Jul 24, 2019 at 3:53 PM Rob Landley <rob at landley.net> wrote:
>
> On 7/23/19 11:50 PM, enh via Toybox wrote:
> >  static void do_tac(int fd, char *name)
> >  {
> >    struct arg_list *list = NULL;
> > -  char *c;
> > +  FILE *fp;
> >
> > -  // Read in lines
> > +  if (fd == -1) {
> > +    perror_msg_raw(name);
> > +    return;
> > +  }
>
> The infrastructure does that if you pass WARN_ONLY in the flags. (Which
> loopfiles() does by default.)
>
> Don't close stdin when they -, it makes the rest of your pipeline unhappy.
> (do_lines() in lib/ checks for this)...
>
> Actually, lemme rewrite this to use do_lines(), via the loopfiles_lines() bridge
> that's already in lib. And use a dlist since that already has a function to
> allocate the wrapper for us and it's trivially reversible...
>
> There, checked in on top of yours. Try now?

even better, thanks.

i did realize though that the no-O_CLOEXEC bit is actually missing
from loopfiles_lines itself. see attached patch.

(one day we'll catch these automatically, but we're not in enforcing
mode yet so this one was just by inspection. see
https://android.googlesource.com/platform/bionic/+/master/docs/fdsan.md
for details of the sanitizer that's in Android Q. this double close
isn't an actual problem here, where nothing runs in between fclose()
and close(), but it's a common problem in long-lived multi-threaded
processes, which is basically everything these days.)

> Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Avoid-double-close-of-fd-in-loopfiles_lines.patch
Type: text/x-patch
Size: 851 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20190724/5ad161a7/attachment-0003.bin>