On Thu, 1 Dec 2016 22:50:20 -0800
Michael Forney <mforney_AT_mforney.org> wrote:
Hey Michael,
> memmove moves a number of bytes, not pointers, so if you passed a
> number of arguments that is larger than the pointer byte size, you
> could end up crashing or skipping the install of a file and
> installing another twice.
well-observed, nice find!
> Also, argv was never decreased to match the moved arguments, so the -t
> parameter was added in the NULL argv slot.
> - memmove(argv - 1, argv, argc);
> + argv = memmove(argv - 1, argv, argc * sizeof(*argv));
I got to admit that this piece of code is really ugly to begin with. We
_must not_ use memmove here as we invoke undefined behaviour, given the
two memory regions overlap.
Also, it's really bad style to call the value "tflag", given it's not
an int but actually a char pointer to the name of the target folder, so
"tflag" should rather be called "target". Same applies to the other
values.
I am wondering if we even need this. I mean, we already "consume" the
target directory in ARGBEGIN ... ARGEND and thus are only left with
sources in argv.
Moreover, I generally question the existence of some flags for install
(1), like -s to strip symbols. Do we really need this? Especially with
non-standardized tools like install(1), we need to be careful not to
swallow the waste that has accumulated over the years.
The usage of this tool has become so complicated that using it properly
becomes harder and harder with the number of options growing.
What are your thoughts? Is the -s flag direly needed for some
applications? Should we just silently ignore it?
Cheers
Laslo
--
Laslo Hunhold <dev_AT_frign.de>
Received on Fri Dec 02 2016 - 13:22:16 CET