Re: cpio improvements and fixes

[Shreenidhi Shedi]

> There's no use to.  x2nrealloc does its job well enough

> without any special hints from the caller.  Besides, this:
>       else
>       {
>           len <<= 1;
>       }

> introduces an integer overlow.

Agree with your other comments except this one.

x2nrealloc in a loop is doing well. This is an attempt to improve it and get rid of loop.

PATH_MAX is 4096 according to limits.h and size_t is 8 bytes which will easily fit.

I did a quick check and "process_copy_pass" function is calling "ds_reset (&output_name, dirname_len);" so unless dirname_len is insanely long (which is not allowed by the system), it won't overflow under any circumstances.

Getting rid of this loop will surely give a good performance boost. Please correct me if I'm wrong here.

Thanks,

Shedi

On Fri, Sep 3, 2021 at 12:27 PM Sergey Poznyakoff <gray@gnu.org.ua> wrote:

Shreenidhi Shedi <yesshedi@gmail.com> ha escrit:

> > [PATCH 03/12] Format specifier fixes
> > Needs more investigation (the use of %zu is not portable)
>
> Shedi: Shall I just do something like this: fptintf(stderr, "nlink =
> %u ...", (unsigned) h->c_nlink, ...); instead?

Allow me some time.

> > [PATCH 07/12] Optimize ds_resize logic
[...]
> Shedi: In that case can we do,

There's no use to.  x2nrealloc does its job well enough
without any special hints from the caller.  Besides, this:
>       else
>       {
>           len <<= 1;
>       }

introduces an integer overlow.

> > [PATCH 08/12] Reformat & refactor userspec.c
> > What's the purpose of this?
> Shedi: Alloca is a deprecated function in C99. Static analyzers throw
> a warning for alloca usage. So, I started modifying code to remove
> alloca usage and ended up refactoring it.

I'll remove it, eventually.

> > [PATCH 11/12] Use strtol instead of atoi
[..]
> > -      io_block_size = atoi (arg);
> > +      io_block_size = (int) strtol (arg, NULL, 10);
>
> Shedi: The next line has a `if (io_block_size < 1)` that should
> suffice right?

It would equally suffice for atoi, so there's no need to replace it.
The main advantage of using strto... over atoi is its error detection
capabilities.  The usual paradigm is:

    char *end;
    long n;

    errno = 0;
    n = strtol (arg, &end, 10);
    if (errno || *end || n < 0 || n > INT_MAX)
      error (...);
    io_block_size = n;

See strtol(3) for details.  In this particular case I don't see any
advantage in using strtol.

Regards,
Sergey