[Toybox] [PATCH] tar: Fix support for long symbolic links

[enh]

https://github.com/landley/toybox/commit/14eee98d9dbe81729437bf333ea413b43e37969a#diff-2c433160b0e21dc043bae1484ee3077bL215
switched to strncpy, but that patch upset clang:

external/toybox/toys/pending/tar.c:549:49: error: '&&' within '||'
[-Werror,-Wlogical-op-parentheses]
    if (filter(TT.excl, TT.hdr.name) || TT.incl && !delete) skippy(TT.hdr.size);
                                     ~~ ~~~~~~~~^~~~~~~~~~

it's way too early in the morning for me to work out what was intended there...

On Wed, Mar 20, 2019 at 8:08 PM Daniel Mentz <danielmentz at google.com> wrote:
>
> On Wed, Mar 20, 2019 at 7:24 PM Rob Landley <rob at landley.net> wrote:
> > On 3/19/19 6:38 PM, Daniel Mentz via Toybox wrote:
> > > -// TODO: does this need NUL terminator?
> > >      if (strlen(lnk) > sizeof(hdr.link))
> >
> > The todo item you erased there was a note to self about checking what happens
> > when a value fits exactly in the field and thus there is no NUL terminator, and
> > whether the read side was handling that correctly. (Similarly, in theory a
>
> If I'm not mistaken, the code on the read side that handles the
> tar.link field is the following:
>
> if (!TT.hdr.link_target && *tar.link)
>   TT.hdr.link_target = xstrndup(tar.link, sizeof(tar.link));
>
> The manpage of strndup reads as follows:
>
> "char *strndup(const char *s, size_t n);
> If s is longer than n, only n bytes are copied, and a terminating null
> byte ('\0') is added."
>
> Hence, if tar.link does not contain a terminating null byte, strndup()
> will add one. So, no, it does not need a NULL terminator.
>
> >
> > >        write_longname(lnk, 'K'); //write longname LINK
> > > -// TODO: this will error_exit() if too long, not truncate.
> > > -    xstrncpy(hdr.link, lnk, sizeof(hdr.link));
> > > +    strncpy(hdr.link, lnk, sizeof(hdr.link));
> >
> > That todo was because it should be strlcpy(), but unfortunately Ulrich Drepper
> > was an asshole and the committee that replaced him is very, very slow. As in
> > this was 2014: https://lwn.net/Articles/612244/
>
> I believe that strncpy is the correct function, since the string does
> not need to be NULL terminated if it's 100 bytes long (i.e.
> sizeof(hdr.link)). If you were to use strlcpy(), you would actually
> lose one useful byte i.e. you wouldn't be able to fit a string that is
> exactly 100 bytes long (you'd truncate it to 99 bytes and add a NULL
> byte that's not required).
> I briefly looked at a tar file generated by GNU tar, and hdr.link
> wasn't NULL terminated there either (I had GNU tar store a symbolic
> link where the target was longer than 100 bytes).