On Thu, 11 Feb 2016 11:45:32 +0000
Dimitris Papastamos <sin_AT_2f30.org> wrote:
> Looks good, thanks! Some minor comments below.
>
> On Thu, Feb 11, 2016 at 12:06:14PM +0100, Mattias Andrée
> wrote:
> > ---
> > LICENSE | 1 +
> > Makefile | 9 ++-
> > README | 1 +
> > TODO | 1 -
> > install.c | 259
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 268 insertions(+), 3 deletions(-)
> > create mode 100644 install.c
>
> Missing manpage.
I'm on it!
>
> > + install.out\
>
> What is this .out?
It cannot be `install` because it conflicts with
install rule. So I decided to append .out (like in a.out)
to make it work.
>
> > +static void
> > +make_dir(char *dir, int was_missing)
> > +{
> > + if (!mkdir(dir, was_missing ? 0755 : mode)) {
> > + if (!was_missing && (lchown(dir,
> > owner, group) < 0))
> > + eprintf("lchmod %s:", dir);
> > + } else if (errno != EEXIST) {
> > + eprintf("mkdir %s:", dir);
> > + }
> > +}
> > +
> > +static void
> > +make_dirs(char *dir, int was_missing)
> > +{
> > + char *p;
> > + for (p = strchr(dir + (dir[0] == '/'), '/');
> > p; p = strchr(p + 1, '/')) {
> > + *p = '\0';
> > + make_dir(dir, was_missing);
> > + *p = '/';
> > + }
> > + make_dir(dir, was_missing);
> > +}
>
> Can we use mkdirp() from libutil?
mkdirp has to be change to allow custom
mode, but it also has to be change to
optioanlly do lchown. I though this was
better because other utilities will probably
not need this.
>
> > +static void
> > +strip(const char *filename)
> > +{
> > + pid_t pid = fork();
> > + switch (pid)
> > + {
>
> Style fix.
Whoops.
>
> > + case -1:
> > + perror("fork");
> > + exit(EXIT_FAILURE);
>
> Just use eprintf(). We don't use EXIT_* in sbase.
OK.
>
> > + if (mflag) {
> > + mode = parsemode(mflag, mode, 0);
> > + if (mode < 0)
> > + return EXIT_FAILURE;
>
> Ditto.
>
OK.
Received on Thu Feb 11 2016 - 13:57:00 CET