bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] libhurdutil: New library containing utils to be used by Guix


From: Justus Winter
Subject: Re: [PATCH] libhurdutil: New library containing utils to be used by Guix.
Date: Thu, 26 May 2016 18:29:44 +0200
User-agent: alot/0.3.8.dev

Hi :)

Quoting manolis837@gmail.com (2016-05-25 17:05:57)
> From: Manolis Ragkousis <manolis837@gmail.com>
> 
> * libhurdutil/hurdutil.h: New file.
> * libhurdutil/settrans.c: New file.
> * libhurdutil/Makefile: New file.
> * utils/Makefile (HURDLIBS, settrans): Use the new library.
> * utils/settrans.c: Update to use the new library.
> * Makefile: (lib-subdirs): Add library.
> ---
>  Makefile               |   2 +-
>  libhurdutil/Makefile   |  28 +++
>  libhurdutil/hurdutil.h |  78 ++++++++
>  libhurdutil/settrans.c | 377 +++++++++++++++++++++++++++++++++++++++
>  utils/Makefile         |   4 +-
>  utils/settrans.c       | 475 
> +++++++++++--------------------------------------
>  6 files changed, 594 insertions(+), 370 deletions(-)
>  create mode 100644 libhurdutil/Makefile
>  create mode 100644 libhurdutil/hurdutil.h
>  create mode 100644 libhurdutil/settrans.c
> 
> diff --git a/Makefile b/Makefile
> index d48baaa..e712767 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -29,7 +29,7 @@ include ./Makeconf
>  lib-subdirs = libshouldbeinlibc libihash libiohelp libports libthreads \
>               libpager libfshelp libdiskfs libtrivfs libps \
>               libnetfs libpipe libstore libhurdbugaddr libftpconn libcons \
> -             libhurd-slab
> +             libhurd-slab libhurdutil
>  
>  # Hurd programs
>  prog-subdirs = auth proc exec term \
> diff --git a/libhurdutil/Makefile b/libhurdutil/Makefile
> new file mode 100644
> index 0000000..2e0e642
> --- /dev/null
> +++ b/libhurdutil/Makefile
> @@ -0,0 +1,28 @@
> +#   Copyright (C) 2016 Free Software Foundation, Inc.
> +#
> +#   This file is part of the GNU Hurd.
> +#
> +#   The GNU Hurd is free software; you can redistribute it and/or
> +#   modify it under the terms of the GNU General Public License as
> +#   published by the Free Software Foundation; either version 2, or (at
> +#   your option) any later version.
> +#
> +#   The GNU Hurd is distributed in the hope that it will be useful, but
> +#   WITHOUT ANY WARRANTY; without even the implied warranty of
> +#   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +#   General Public License for more details.
> +#
> +#   You should have received a copy of the GNU General Public License
> +#   along with this program; if not, write to the Free Software
> +#   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.

This is an old license template, the current one doesn't list a
snailmail address, but some URL.

> +
> +dir := libhurdutil
> +makemode := library
> +
> +libname := libhurdutil
> +SRCS = settrans.c
> +installhdrs = hurdutil.h
> +
> +OBJS = $(SRCS:.c=.o)
> +
> +include ../Makeconf
> diff --git a/libhurdutil/hurdutil.h b/libhurdutil/hurdutil.h
> new file mode 100644
> index 0000000..5786cff
> --- /dev/null
> +++ b/libhurdutil/hurdutil.h
> @@ -0,0 +1,78 @@
> +/* hurdutil.h - Hurd utils interface.
> +   Copyright (C) 2016 Free Software Foundation, Inc.
> +   Written by Manolis Fragkiskos Ragkousis <manolis837@gmail.com>.
> +
> +   This file is part of the GNU Hurd.
> +
> +   The GNU Hurd is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU General Public License as
> +   published by the Free Software Foundation; either version 2, or (at
> +   your option) any later version.
> +
> +   The GNU Hurd is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with the GNU Hurd; if not, write to the Free Software
> +   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.  */
> +
> +#ifndef _HURD_UTIL_H
> +#define _HURD_UTIL_H
> +
> +#include <errno.h>
> +#include <sys/types.h>
> +#include <limits.h>
> +#include <stdint.h>
> +#include <stddef.h>
> +
> +#include <hurd/fsys.h>
> +

As Guillem noted, we might want to change the way options are passed
into the library.  Let's see...

> +struct settrans_flags
> +{
> +  /* The name of the node we're putting the translator on. */
> +  char *node_name;
> +
> +  /* Flags to pass to file_set_translator.  */
> +  int lookup_flags;
> +  int goaway_flags;

These two contain ORed flags.

> +  /* Various option flags.  */
> +  int passive;
> +  int active;
> +  int keep_active;
> +  int pause;
> +  int kill_active;
> +  int orphan;
> +  int start;
> +  int stack;
> +  int excl;
> +  int timeout;

These are boolean, and the idiomatic way would be to OR some constants
containing powers of two together.  Or use a bitfield, that might be
more modern, I don't know.

> +  char *pid_file;
> +  char *underlying_node_name;
> +  int underlying_lookup_flags;
> +  char **chroot_command;
> +  char *chroot_chdir;
> +
> +  /* The translator's arg vector, in '\0' separated format.  */
> +  char *argz;
> +  size_t argz_len;
> +};

So only using bitflags is not enough, we have plenty of other kind of
options.  Maybe we shouldn't call it 'settrans_flags' then, maybe
'settrans_context'?

Guillem wrote that he doesn't like that the definition of that struct
is public, and he'd create setters/getters for that.  That is a matter
of taste I guess, but I don't mind exposing that, and I would mind
writing all these setters that then in turn get exposed in the
interface.

If you want to verify that your interface is generic enough to be used
by other applications, you could try to port our mount to it.  This is
optional, you don't need to do it, and I haven't looked at mount for a
while and don't know if it even makes sense, just a thought.

> +typedef struct settrans_flags *settrans_flags_t;

Don't do that.  You cannot see from reading 'foo_t' that it is a
pointer-type (or is it!?).  Just use 'struct foo *'.

> +
> +/* Initialize the flags to be used. */
> +void settrans_flags_init (settrans_flags_t flags);
> +
> +/* Release the memory allocated. */
> +void settrans_flags_cleanup (settrans_flags_t flags);
> +
> +/* Create the struct containing the flags and initialize them.
> +   If a memory allocation error occurs, ENOMEM is returned,
> +   otherwise 0.*/
> +error_t settrans_flags_create (settrans_flags_t *flags);
> +
> +/* Set a translator according to the flags passed. On success return 0. */
> +error_t settrans(settrans_flags_t flags);
> +
> +#endif /* _HURD_UTIL_H */
> diff --git a/libhurdutil/settrans.c b/libhurdutil/settrans.c
> new file mode 100644
> index 0000000..eeaa964
> --- /dev/null
> +++ b/libhurdutil/settrans.c
> @@ -0,0 +1,377 @@
> +/* settrans.c - Set a file's translator.
> +
> +   Copyright (C) 1995,96,97,98,2001,02,13,14,16
> +   Free Software Foundation, Inc.
> +   Written by Miles Bader <miles@gnu.org>
> +   Written by Manolis Fragkiskos Ragkousis <manolis837@gmail.com>.
> +
> +   This file is part of the GNU Hurd.
> +
> +   The GNU Hurd is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU General Public License as
> +   published by the Free Software Foundation; either version 2, or (at
> +   your option) any later version.
> +
> +   The GNU Hurd is distributed in the hope that it will be useful, but
> +   WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with the GNU Hurd; if not, write to the Free Software
> +   Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.  */
> +
> +#include <errno.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <assert.h>
> +#include <fcntl.h>
> +#include <argp.h>
> +#include <unistd.h>
> +#include <sys/wait.h>
> +
> +#include <hurd.h>
> +#include <error.h>
> +#include <argz.h>
> +#include <hurd/fshelp.h>
> +#include <hurd/lookup.h>
> +
> +
> +#include "hurdutil.h"
> +
> +#define DEFAULT_TIMEOUT 60
> +
> +/* Authentication of the current process.  */
> +uid_t *uids;
> +gid_t *gids;
> +size_t uids_len, gids_len;
> +
> +/* Initialize and populate the uids and gids vectors.  */
> +error_t
> +get_credentials (void)
> +{
> +  /* Fetch uids...  */
> +  uids_len = geteuids (0, 0);
> +  if (uids_len < 0)
> +    return errno;
> +
> +  uids = malloc (uids_len * sizeof (uid_t));
> +  if (! uids)
> +    return ENOMEM;
> +
> +  uids_len = geteuids (uids_len, uids);
> +  if (uids_len < 0)
> +    return errno;
> +
> +  /* ... and gids.  */
> +  gids_len = getgroups (0, 0);
> +  if (gids_len < 0)
> +    return errno;
> +
> +  gids = malloc (gids_len * sizeof (gid_t));
> +  if (! uids)
> +    return ENOMEM;
> +
> +  gids_len = getgroups (gids_len, gids);
> +  if (gids_len < 0)
> +    return errno;
> +
> +  return 0;
> +}
> +
> 
> +/* ---------------------------------------------------------------- */
> +
> +void
> +settrans_flags_init (settrans_flags_t flags)
> +{
> +  flags->node_name = 0;
> +
> +  flags->lookup_flags = O_NOTRANS;
> +  flags->goaway_flags = 0;
> +
> +  flags->passive = 0;
> +  flags->active = 0;
> +  flags->keep_active = 0;
> +  flags->pause = 0;
> +  flags->kill_active = 0;
> +  flags->orphan = 0;
> +  flags->start = 0;
> +  flags->stack = 0;
> +  flags->excl = 0;
> +  flags->timeout = DEFAULT_TIMEOUT * 1000; /* ms */
> +  flags->pid_file = 0;
> +  flags->underlying_node_name = NULL;
> +  flags->chroot_command = 0;
> +  flags->chroot_chdir = "/";
> +
> +  flags->argz;
> +  flags->argz_len;
> +}
> +
> +void
> +settrans_flags_cleanup (settrans_flags_t flags)
> +{
> +  free(flags);
> +}
> +
> +error_t
> +settrans_flags_create (settrans_flags_t *flags)
> +{
> +  *flags = malloc (sizeof (struct settrans_flags));
> +  if (*flags == NULL)
> +    return ENOMEM;
> +
> +  settrans_flags_init(*flags);
> +
> +  return 0;
> +}
> +
> +error_t settrans(settrans_flags_t flags)
> +{
> +  error_t err;
> +
> +  /* The filesystem node we're putting a translator on.  */
> +  char *node_name = flags->node_name;
> +  file_t node;
> +
> +  /* The translator's arg vector, in '\0' separated format.  */
> +  char *argz = flags->argz;
> +  size_t argz_len = flags->argz_len;
> +
> +  /* The control port for any active translator we start up.  */
> +  fsys_t active_control = MACH_PORT_NULL;
> +
> +  /* Flags to pass to file_set_translator.  */
> +  int active_flags = 0;
> +  int passive_flags = 0;
> +  int lookup_flags = flags->lookup_flags;
> +  int goaway_flags = flags->goaway_flags;
> +
> +  /* Various option flags.  */
> +  int passive = flags->passive;
> +  int active = flags->active;
> +  int keep_active = flags->keep_active;
> +  int pause = flags->pause;
> +  int kill_active = flags->kill_active;
> +  int orphan = flags->orphan;
> +  int start = flags->start;
> +  int stack = flags->stack;
> +  char *pid_file = flags->pid_file;
> +  int excl = flags->excl;
> +  int timeout = flags->timeout; /* ms */
> +  char *underlying_node_name = flags->underlying_node_name;
> +  int underlying_lookup_flags = flags->underlying_lookup_flags;
> +  char **chroot_command = flags->chroot_command;
> +  char *chroot_chdir = flags->chroot_chdir;
> +
> +  if (stack)
> +    {
> +      underlying_node_name = node_name;
> +      underlying_lookup_flags = lookup_flags && ~O_NOTRANS;
> +    }
> +  else
> +    underlying_lookup_flags = lookup_flags;
> +
> +
> +  if (!active && !passive && !chroot_command)
> +    passive = 1;               /* By default, set the passive translator.  */
> +
> +  if (passive)
> +    passive_flags = FS_TRANS_SET | (excl ? FS_TRANS_EXCL : 0);
> +  if (active)
> +    active_flags = FS_TRANS_SET | (excl ? FS_TRANS_EXCL : 0)
> +      | (orphan ? FS_TRANS_ORPHAN : 0);
> +
> +  if (passive && !active)
> +    {
> +      /* When setting just the passive, decide what to do with any active.  
> */
> +      if (kill_active)
> +        /* Make it go away.  */
> +        active_flags = FS_TRANS_SET;
> +      else if (! keep_active)
> +        /* Ensure that there isn't one.  */
> +        active_flags = FS_TRANS_SET | FS_TRANS_EXCL;
> +    }
> +
> +  if (start)
> +    {
> +      /* Retrieve the passive translator record in argz.  */
> +      mach_port_t node = file_name_lookup (node_name, lookup_flags, 0);
> +      if (node == MACH_PORT_NULL)
> +        error (4, errno, "%s", node_name);
> +
> +      char buf[1024];
> +      argz = buf;
> +      argz_len = sizeof (buf);
> +
> +      err = file_get_translator (node, &argz, &argz_len);
> +      if (err == EINVAL)
> +        error (4, 0, "%s: no passive translator record found", node_name);
> +      if (err)
> +        error (4, err, "%s", node_name);
> +
> +      mach_port_deallocate (mach_task_self (), node);
> +    }
> +
> +  if ((active || chroot_command) && argz_len > 0)
> +    {
> +      /* Error during file lookup; we use this to avoid duplicating error
> +         messages.  */
> +      error_t open_err = 0;
> +
> +      /* The callback to start_translator opens NODE as a side effect.  */
> +      error_t open_node (int flags,
> +                         mach_port_t *underlying,
> +                         mach_msg_type_name_t *underlying_type,
> +                         task_t task, void *cookie)
> +      {
> +        if (pause)
> +          {
> +            fprintf (stderr, "Translator pid: %d\nPausing...",
> +                     task2pid (task));
> +            getchar ();
> +          }
> +
> +        if (pid_file != NULL)
> +          {
> +            FILE *h;
> +            h = fopen (pid_file, "w");
> +            if (h == NULL)
> +              error (4, errno, "Failed to open pid file");
> +
> +            fprintf (h, "%i\n", task2pid (task));
> +            fclose (h);
> +          }
> +
> +        node = file_name_lookup (node_name, flags | lookup_flags, 0666);
> +        if (node == MACH_PORT_NULL)
> +          {
> +            open_err = errno;
> +            return open_err;
> +          }
> +
> +        if (underlying_node_name)
> +          {
> +            *underlying = file_name_lookup (underlying_node_name,
> +                                            flags | underlying_lookup_flags,
> +                                            0666);
> +            if (! MACH_PORT_VALID (*underlying))
> +              {
> +                /* For the error message.  */
> +                node_name = underlying_node_name;
> +                open_err = errno;
> +                return open_err;
> +              }
> +          }
> +        else
> +          *underlying = node;
> +        *underlying_type = MACH_MSG_TYPE_COPY_SEND;
> +
> +        return 0;
> +      }
> +      err = fshelp_start_translator (open_node, NULL, argz, argz, argz_len,
> +                                     timeout, &active_control);
> +      if (err)
> +        /* If ERR is due to a problem opening the translated node, we print
> +           that name, otherwise, the name of the translator.  */
> +        error(4, err, "%s", (err == open_err) ? node_name : argz);
> +    }
> +  else
> +    {
> +      node = file_name_lookup(node_name, lookup_flags, 0666);
> +      if (node == MACH_PORT_NULL)
> +        error(1, errno, "%s", node_name);
> +    }
> +
> +  if (active || passive)
> +    {
> +      err = file_set_translator (node,
> +                                 passive_flags, active_flags, goaway_flags,
> +                                 argz, argz_len,
> +                                 active_control, MACH_MSG_TYPE_COPY_SEND);
> +      if (err)
> +        {
> +          error (5, err, "%s", node_name);
> +        }
> +
> +    }
> +
> +  if (chroot_command)

This is the underappreciated part of settrans.  It is used e.g. in our
fakeroot, or in remap.  I'm curious if that makes sense in the library
setting, but it may very well be nice, e.g. to implement chroot(2) on
top of a proxying translator (I'm currently working on that).

> +    {
> +      pid_t child;
> +      int status;
> +      switch ((child = fork ()))
> +        {
> +        case -1:
> +          error (6, errno, "fork");
> +
> +        case 0:; /* Child.  */
> +          /* We will act as the parent filesystem would for a lookup
> +             of the active translator's root node, then use this port
> +             as our root directory while we exec the command.  */
> +
> +          char retry_name[1024];       /* XXX */
> +          retry_type do_retry;
> +          mach_port_t root;
> +          file_t executable;
> +          char *prefixed_name;
> +
> +          err = get_credentials ();
> +          if (err)
> +            error (6, err, "getting credentials");
> +
> +          err = fsys_getroot (active_control,
> +                              MACH_PORT_NULL, MACH_MSG_TYPE_COPY_SEND,
> +                              uids, uids_len, gids, gids_len, 0,
> +                              &do_retry, retry_name, &root);
> +          mach_port_deallocate (mach_task_self (), active_control);
> +          if (err)
> +            error (6, err, "fsys_getroot");
> +          err = hurd_file_name_lookup_retry (&_hurd_ports_use, &getdport, 0,
> +                                             do_retry, retry_name, 0, 0,
> +                                             &root);
> +          if (err)
> +            error (6, err, "cannot resolve root port");
> +
> +          if (setcrdir (root))
> +            error (7, errno, "cannot install root port");
> +          mach_port_deallocate (mach_task_self (), root);
> +          if (chdir (chroot_chdir))
> +            error (8, errno, "%s", chroot_chdir);
> +
> +          /* Lookup executable in PATH.  */
> +          executable = file_name_path_lookup (chroot_command[0],
> +                                              getenv ("PATH"),
> +                                              O_EXEC, 0,
> +                                              &prefixed_name);
> +          if (MACH_PORT_VALID (executable))
> +            {
> +              err = mach_port_deallocate (mach_task_self (), executable);
> +              assert_perror (err);
> +              if (prefixed_name)
> +                chroot_command[0] = prefixed_name;
> +            }
> +
> +          execvp (chroot_command[0], chroot_command);
> +          error (8, errno, "cannot execute %s", chroot_command[0]);
> +          break;
> +
> +        default: /* Parent.  */
> +          if (waitpid (child, &status, 0) != child)
> +            error (8, errno, "waitpid on %d", child);
> +
> +          err = fsys_goaway (active_control, goaway_flags);
> +          if (err && err != EBUSY)
> +            error (9, err, "fsys_goaway");
> +
> +          if (WIFSIGNALED (status))
> +            error (WTERMSIG (status) + 128, 0,
> +                   "%s for child %d", strsignal (WTERMSIG (status)), child);
> +          if (WEXITSTATUS (status) != 0)
> +            error (WEXITSTATUS (status), 0,
> +                   "Error %d for child %d", WEXITSTATUS (status), child);
> +        }
> +    }
> +  return 0;
> +}
> diff --git a/utils/Makefile b/utils/Makefile
> index d2ef9e8..be10cf6 100644
> --- a/utils/Makefile
> +++ b/utils/Makefile
> @@ -34,7 +34,7 @@ SRCS = shd.c ps.c settrans.c syncfs.c showtrans.c addauth.c 
> rmauth.c \
>         nullauth.c match-options.c msgids.c rpcscan.c
>  
>  OBJS = $(filter-out %.sh,$(SRCS:.c=.o))
> -HURDLIBS = ps ihash store fshelp ports ftpconn shouldbeinlibc
> +HURDLIBS = ps ihash store fshelp ports ftpconn shouldbeinlibc hurdutil
>  LDLIBS += -lpthread
>  login-LDLIBS = -lutil -lcrypt
>  addauth-LDLIBS = -lcrypt
> @@ -60,7 +60,7 @@ storeinfo storecat storeread: ../libstore/libstore.a
>  ftpcp ftpdir: ../libftpconn/libftpconn.a
>  mount umount: ../libihash/libihash.a
>  settrans: ../libfshelp/libfshelp.a ../libihash/libihash.a \
> -       ../libports/libports.a
> +       ../libports/libports.a ../libhurdutil/libhurdutil.a
>  ps w ids settrans syncfs showtrans fsysopts storeinfo login vmstat portinfo \
>    devprobe vminfo addauth rmauth setauth unsu ftpcp ftpdir storeread \
>    storecat msgport mount umount nullauth rpctrace: \
> diff --git a/utils/settrans.c b/utils/settrans.c
> index ee7cba5..14dd520 100644
> --- a/utils/settrans.c
> +++ b/utils/settrans.c
> @@ -3,6 +3,7 @@
>     Copyright (C) 1995,96,97,98,2001,02,13,14
>       Free Software Foundation, Inc.
>     Written by Miles Bader <miles@gnu.org>
> +   Revised by Manolis Fragkiskos Ragkousis <manolis837@gmail.com>.
>  
>     This program is free software; you can redistribute it and/or
>     modify it under the terms of the GNU General Public License as
> @@ -38,11 +39,10 @@
>  #include <hurd/lookup.h>
>  #include <hurd/fsys.h>
>  
> +#include <hurd/hurdutil.h>
>  
>  const char *argp_program_version = STANDARD_HURD_VERSION (settrans);
>  
> -#define DEFAULT_TIMEOUT 60
> -
>  #define _STRINGIFY(arg) #arg
>  #define STRINGIFY(arg) _STRINGIFY (arg)
>  
> @@ -93,44 +93,6 @@ static char *args_doc = "NODE [TRANSLATOR ARG...]";
>  static char *doc = "Set the passive/active translator on NODE."
>  "\vBy default the passive translator is set.";
>  
> 
> -/* Authentication of the current process.  */
> -uid_t *uids;
> -gid_t *gids;
> -size_t uids_len, gids_len;
> -
> -/* Initialize and populate the uids and gids vectors.  */
> -error_t
> -get_credentials (void)
> -{
> -  /* Fetch uids...  */
> -  uids_len = geteuids (0, 0);
> -  if (uids_len < 0)
> -    return errno;
> -
> -  uids = malloc (uids_len * sizeof (uid_t));
> -  if (! uids)
> -    return ENOMEM;
> -
> -  uids_len = geteuids (uids_len, uids);
> -  if (uids_len < 0)
> -    return errno;
> -
> -  /* ... and gids.  */
> -  gids_len = getgroups (0, 0);
> -  if (gids_len < 0)
> -    return errno;
> -
> -  gids = malloc (gids_len * sizeof (gid_t));
> -  if (! uids)
> -    return ENOMEM;
> -
> -  gids_len = getgroups (gids_len, gids);
> -  if (gids_len < 0)
> -    return errno;
> -
> -  return 0;
> -}
> -
> 
>  /* ---------------------------------------------------------------- */
>  
>  int
> @@ -138,343 +100,122 @@ main(int argc, char *argv[])
>  {
>    error_t err;
>  
> -  /* The filesystem node we're putting a translator on.  */
> -  char *node_name = 0;
> -  file_t node;
> -
> -  /* The translator's arg vector, in '\0' separated format.  */
> -  char *argz = 0;
> -  size_t argz_len = 0;
> -
> -  /* The control port for any active translator we start up.  */
> -  fsys_t active_control = MACH_PORT_NULL;
> -
> -  /* Flags to pass to file_set_translator.  */
> -  int active_flags = 0;
> -  int passive_flags = 0;
> -  int lookup_flags = O_NOTRANS;
> -  int goaway_flags = 0;
> -
> -  /* Various option flags.  */
> -  int passive = 0, active = 0, keep_active = 0, pause = 0, kill_active = 0,
> -      orphan = 0;
> -  int start = 0;
> -  int stack = 0;
> -  char *pid_file = NULL;
> -  int excl = 0;
> -  int timeout = DEFAULT_TIMEOUT * 1000; /* ms */
> -  char *underlying_node_name = NULL;
> -  int underlying_lookup_flags;
> -  char **chroot_command = 0;
> -  char *chroot_chdir = "/";
> +  /* The flags to be used to create the translator. */
> +  settrans_flags_t flags;
> +
> +  settrans_flags_create(&flags);
>  
>    /* Parse our options...  */
>    error_t parse_opt (int key, char *arg, struct argp_state *state)

If you are practically changing every line of this function, and move
all the flags out of main's scope anyway, this is the time to move it
out of main's body, and turn it into a normal function.

> -    {
> -      switch (key)
> -       {
> -       case ARGP_KEY_ARG:
> -         if (state->arg_num == 0)
> -           node_name = arg;
> -         else                  /* command */
> -           {
> -             if (start)
> -               argp_error (state, "both --start and TRANSLATOR given");
> -
> -             error_t err =
> -               argz_create (state->argv + state->next - 1, &argz, &argz_len);
> -             if (err)
> -               error(3, err, "Can't create options vector");
> -             state->next = state->argc; /* stop parsing */
> -           }
> -         break;
> -
> -       case ARGP_KEY_NO_ARGS:
> -         argp_usage (state);
> -         return EINVAL;
> -
> -       case 'a': active = 1; break;
> -       case 's':
> -         start = 1;
> -         active = 1;   /* start implies active */
> -         break;
> -       case OPT_STACK:
> -         stack = 1;
> -         active = 1;   /* stack implies active */
> -         orphan = 1;   /* stack implies orphan */
> -         break;
> -       case 'p': passive = 1; break;
> -       case 'k': keep_active = 1; break;
> -       case 'g': kill_active = 1; break;
> -       case 'x': excl = 1; break;
> -       case 'P': pause = 1; break;
> -       case 'F':
> -         pid_file = strdup (arg);
> -         if (pid_file == NULL)
> -           error(3, ENOMEM, "Failed to duplicate argument");
> -         break;
> -
> -       case 'o': orphan = 1; break;
> -       case 'U':
> -         underlying_node_name = strdup (arg);
> -         if (underlying_node_name == NULL)
> -           error(3, ENOMEM, "Failed to duplicate argument");
> -         break;
> -
> -       case 'C':
> -         if (chroot_command)
> -           {
> -             argp_error (state, "--chroot given twice");
> -             return EINVAL;
> -           }
> -         chroot_command = &state->argv[state->next];
> -         while (state->next < state->argc)
> -           {
> -             if (!strcmp (state->argv[state->next], "--"))
> -               {
> -                 state->argv[state->next++] = 0;
> -                 if (chroot_command[0] == 0)
> -                   {
> -                     argp_error (state,
> -                                 "--chroot must be followed by a command");
> -                     return EINVAL;
> -                   }
> -                 return 0;
> -               }
> -             ++state->next;
> -           }
> -         argp_error (state, "--chroot command must be terminated with `--'");
> -         return EINVAL;
> -
> -       case OPT_CHROOT_CHDIR:
> -         if (arg[0] != '/')
> -           argp_error (state, "--chroot-chdir must be absolute");
> -         chroot_chdir = arg;
> -         break;
> -
> -       case 'c': lookup_flags |= O_CREAT; break;
> -       case 'L': lookup_flags &= ~O_NOTRANS; break;
> -
> -       case 'R': goaway_flags |= FSYS_GOAWAY_RECURSE; break;
> -       case 'S': goaway_flags |= FSYS_GOAWAY_NOSYNC; break;
> -       case 'f': goaway_flags |= FSYS_GOAWAY_FORCE; break;
> -
> -         /* Use atof so the user can specifiy fractional timeouts.  */
> -       case 't': timeout = atof (arg) * 1000.0; break;
> -
> -       default:
> -         return ARGP_ERR_UNKNOWN;
> -       }
> -      return 0;
> -    }
> +  {
> +    switch (key)
> +      {
> +      case ARGP_KEY_ARG:
> +        if (state->arg_num == 0)
> +          flags->node_name = arg;
> +        else                   /* command */
> +          {
> +            if (flags->start)
> +              argp_error (state, "both --start and TRANSLATOR given");
> +
> +            error_t err =
> +              argz_create (state->argv + state->next - 1, &flags->argz, 
> &flags->argz_len);
> +            if (err)
> +              error(3, err, "Can't create options vector");
> +            state->next = state->argc; /* stop parsing */
> +          }
> +        break;
> +
> +      case ARGP_KEY_NO_ARGS:
> +        argp_usage (state);
> +        return EINVAL;
> +
> +      case 'a': flags->active = 1; break;
> +      case 's':
> +        flags->start = 1;
> +        flags->active = 1;     /* start implies active */
> +        break;
> +      case OPT_STACK:
> +        flags->stack = 1;
> +        flags->active = 1;   /* stack implies active */
> +        flags->orphan = 1;   /* stack implies orphan */
> +        break;
> +      case 'p': flags->passive = 1; break;
> +      case 'k': flags->keep_active = 1; break;
> +      case 'g': flags->kill_active = 1; break;
> +      case 'x': flags->excl = 1; break;
> +      case 'P': flags->pause = 1; break;
> +      case 'F':
> +        flags->pid_file = strdup (arg);
> +        if (flags->pid_file == NULL)
> +          error(3, ENOMEM, "Failed to duplicate argument");
> +        break;
> +
> +      case 'o': flags->orphan = 1; break;
> +      case 'U':
> +        flags->underlying_node_name = strdup (arg);
> +        if (flags->underlying_node_name == NULL)
> +          error(3, ENOMEM, "Failed to duplicate argument");
> +        break;
> +
> +      case 'C':
> +        if (flags->chroot_command)
> +          {
> +            argp_error (state, "--chroot given twice");
> +            return EINVAL;
> +          }
> +        flags->chroot_command = &state->argv[state->next];
> +        while (state->next < state->argc)
> +          {
> +            if (!strcmp (state->argv[state->next], "--"))
> +              {
> +                state->argv[state->next++] = 0;
> +                if (flags->chroot_command[0] == 0)
> +                  {
> +                    argp_error (state,
> +                                "--chroot must be followed by a command");
> +                    return EINVAL;
> +                  }
> +                return 0;
> +              }
> +            ++state->next;
> +          }
> +        argp_error (state, "--chroot command must be terminated with `--'");
> +        return EINVAL;
> +
> +      case OPT_CHROOT_CHDIR:
> +        if (arg[0] != '/')
> +          argp_error (state, "--chroot-chdir must be absolute");
> +        flags->chroot_chdir = arg;
> +        break;
> +
> +      case 'c': flags->lookup_flags |= O_CREAT; break;
> +      case 'L': flags->lookup_flags &= ~O_NOTRANS; break;
> +
> +      case 'R': flags->goaway_flags |= FSYS_GOAWAY_RECURSE; break;
> +      case 'S': flags->goaway_flags |= FSYS_GOAWAY_NOSYNC; break;
> +      case 'f': flags->goaway_flags |= FSYS_GOAWAY_FORCE; break;
> +
> +        /* Use atof so the user can specifiy fractional timeouts.  */
> +      case 't': flags->timeout = atof (arg) * 1000.0; break;
> +
> +      default:
> +        return ARGP_ERR_UNKNOWN;
> +      }
> +    return 0;
> +  }
>    struct argp argp = {options, parse_opt, args_doc, doc};
>  
>    argp_parse (&argp, argc, argv, ARGP_IN_ORDER, 0, 0);
>  
> -  if (stack)
> -    {
> -      underlying_node_name = node_name;
> -      underlying_lookup_flags = lookup_flags && ~O_NOTRANS;
> -    }
> -  else
> -    underlying_lookup_flags = lookup_flags;
> -
> -  if (!active && !passive && !chroot_command)
> -    passive = 1;               /* By default, set the passive translator.  */
> -
> -  if (passive)
> -    passive_flags = FS_TRANS_SET | (excl ? FS_TRANS_EXCL : 0);
> -  if (active)
> -    active_flags = FS_TRANS_SET | (excl ? FS_TRANS_EXCL : 0)
> -                  | (orphan ? FS_TRANS_ORPHAN : 0);
> -
> -  if (passive && !active)
> -    {
> -      /* When setting just the passive, decide what to do with any active.  
> */
> -      if (kill_active)
> -       /* Make it go away.  */
> -       active_flags = FS_TRANS_SET;
> -      else if (! keep_active)
> -       /* Ensure that there isn't one.  */
> -       active_flags = FS_TRANS_SET | FS_TRANS_EXCL;
> -    }
> -
> -  if (start)
> -    {
> -      /* Retrieve the passive translator record in argz.  */
> -      mach_port_t node = file_name_lookup (node_name, lookup_flags, 0);
> -      if (node == MACH_PORT_NULL)
> -       error (4, errno, "%s", node_name);
> -
> -      char buf[1024];
> -      argz = buf;
> -      argz_len = sizeof (buf);
> -
> -      err = file_get_translator (node, &argz, &argz_len);
> -      if (err == EINVAL)
> -       error (4, 0, "%s: no passive translator record found", node_name);
> -      if (err)
> -       error (4, err, "%s", node_name);
> -
> -      mach_port_deallocate (mach_task_self (), node);
> -    }
> -
> -  if ((active || chroot_command) && argz_len > 0)
> -    {
> -      /* Error during file lookup; we use this to avoid duplicating error
> -        messages.  */
> -      error_t open_err = 0;
> -
> -      /* The callback to start_translator opens NODE as a side effect.  */
> -      error_t open_node (int flags,
> -                        mach_port_t *underlying,
> -                        mach_msg_type_name_t *underlying_type,
> -                        task_t task, void *cookie)
> -       {
> -         if (pause)
> -           {
> -             fprintf (stderr, "Translator pid: %d\nPausing...",
> -                      task2pid (task));
> -             getchar ();
> -           }
> -
> -         if (pid_file != NULL)
> -           {
> -             FILE *h;
> -             h = fopen (pid_file, "w");
> -             if (h == NULL)
> -               error (4, errno, "Failed to open pid file");
> -
> -             fprintf (h, "%i\n", task2pid (task));
> -             fclose (h);
> -           }
> -
> -         node = file_name_lookup (node_name, flags | lookup_flags, 0666);
> -         if (node == MACH_PORT_NULL)
> -           {
> -             open_err = errno;
> -             return open_err;
> -           }
> -
> -         if (underlying_node_name)
> -           {
> -             *underlying = file_name_lookup (underlying_node_name,
> -                                             flags | underlying_lookup_flags,
> -                                             0666);
> -             if (! MACH_PORT_VALID (*underlying))
> -               {
> -                 /* For the error message.  */
> -                 node_name = underlying_node_name;
> -                 open_err = errno;
> -                 return open_err;
> -               }
> -           }
> -         else
> -           *underlying = node;
> -         *underlying_type = MACH_MSG_TYPE_COPY_SEND;
> -
> -         return 0;
> -       }
> -      err = fshelp_start_translator (open_node, NULL, argz, argz, argz_len,
> -                                    timeout, &active_control);
> -      if (err)
> -       /* If ERR is due to a problem opening the translated node, we print
> -          that name, otherwise, the name of the translator.  */
> -       error(4, err, "%s", (err == open_err) ? node_name : argz);
> -    }
> -  else
> -    {
> -      node = file_name_lookup(node_name, lookup_flags, 0666);
> -      if (node == MACH_PORT_NULL)
> -       error(1, errno, "%s", node_name);
> -    }
> -
> -  if (active || passive)
> +  err = settrans(flags);
> +  if (err)
>      {
> -      err = file_set_translator (node,
> -                                passive_flags, active_flags, goaway_flags,
> -                                argz, argz_len,
> -                                active_control, MACH_MSG_TYPE_COPY_SEND);
> -      if (err)
> -       error (5, err, "%s", node_name);
> +      settrans_flags_cleanup(flags);
> +      error(1, err, "Could not set translator");
>      }
>  
> -  if (chroot_command)
> -    {
> -      pid_t child;
> -      int status;
> -      switch ((child = fork ()))
> -       {
> -       case -1:
> -         error (6, errno, "fork");
> -
> -       case 0:; /* Child.  */
> -         /* We will act as the parent filesystem would for a lookup
> -            of the active translator's root node, then use this port
> -            as our root directory while we exec the command.  */
> -
> -         char retry_name[1024];        /* XXX */
> -         retry_type do_retry;
> -         mach_port_t root;
> -         file_t executable;
> -         char *prefixed_name;
> -
> -         err = get_credentials ();
> -         if (err)
> -           error (6, err, "getting credentials");
> -
> -         err = fsys_getroot (active_control,
> -                             MACH_PORT_NULL, MACH_MSG_TYPE_COPY_SEND,
> -                             uids, uids_len, gids, gids_len, 0,
> -                             &do_retry, retry_name, &root);
> -         mach_port_deallocate (mach_task_self (), active_control);
> -         if (err)
> -           error (6, err, "fsys_getroot");
> -         err = hurd_file_name_lookup_retry (&_hurd_ports_use, &getdport, 0,
> -                                            do_retry, retry_name, 0, 0,
> -                                            &root);
> -         if (err)
> -           error (6, err, "cannot resolve root port");
> -
> -         if (setcrdir (root))
> -           error (7, errno, "cannot install root port");
> -         mach_port_deallocate (mach_task_self (), root);
> -         if (chdir (chroot_chdir))
> -           error (8, errno, "%s", chroot_chdir);
> -
> -         /* Lookup executable in PATH.  */
> -         executable = file_name_path_lookup (chroot_command[0],
> -                                             getenv ("PATH"),
> -                                             O_EXEC, 0,
> -                                             &prefixed_name);
> -         if (MACH_PORT_VALID (executable))
> -           {
> -             err = mach_port_deallocate (mach_task_self (), executable);
> -             assert_perror (err);
> -             if (prefixed_name)
> -               chroot_command[0] = prefixed_name;
> -           }
> -
> -         execvp (chroot_command[0], chroot_command);
> -         error (8, errno, "cannot execute %s", chroot_command[0]);
> -         break;
> -
> -       default: /* Parent.  */
> -         if (waitpid (child, &status, 0) != child)
> -           error (8, errno, "waitpid on %d", child);
> -
> -         err = fsys_goaway (active_control, goaway_flags);
> -         if (err && err != EBUSY)
> -           error (9, err, "fsys_goaway");
> -
> -         if (WIFSIGNALED (status))
> -           error (WTERMSIG (status) + 128, 0,
> -                  "%s for child %d", strsignal (WTERMSIG (status)), child);
> -         if (WEXITSTATUS (status) != 0)
> -           error (WEXITSTATUS (status), 0,
> -                  "Error %d for child %d", WEXITSTATUS (status), child);
> -       }
> -    }
> +  settrans_flags_cleanup(flags);
>  
>    return 0;
>  }
> -- 
> 2.8.2
> 


Thanks :)
Justus



reply via email to

[Prev in Thread] Current Thread [Next in Thread]