On Thu, 1 Dec 2016 22:50:20 -0800 Michael Forney <[email protected]> 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 <[email protected]>
