Re: [dev] [sbase][PATCH] install

From: Dimitris Papastamos <sin_AT_2f30.org>
Date: Thu, 11 Feb 2016 11:45:32 +0000

Looks good, thanks! Some minor comments below.

On Thu, Feb 11, 2016 at 12:06:14PM +0100, Mattias Andrée wrote:
> New command. Includes the flags:
>
> -s strip binary
> -d create directory
> -D create missing directories
> -t DIR target directory
> -m MODE permission bits
> -o USER set owner
> -g GROUP set group
>
> Installed files are copied, and default mode is 755.
>
> Signed-off-by: Mattias Andrée <maandree_AT_kth.se>
> ---
> 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.

> + install.out\

What is this .out?

> +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?

> +static void
> +strip(const char *filename)
> +{
> + pid_t pid = fork();
> + switch (pid)
> + {

Style fix.

> + case -1:
> + perror("fork");
> + exit(EXIT_FAILURE);

Just use eprintf(). We don't use EXIT_* in sbase.

> + if (mflag) {
> + mode = parsemode(mflag, mode, 0);
> + if (mode < 0)
> + return EXIT_FAILURE;

Ditto.
Received on Thu Feb 11 2016 - 12:45:32 CET