Re: [Libguestfs] [PATCH v3 2/6] daemon: Split out command() functions and CLEANUP_* macros into separate files.

On Thursday 21 January 2016 15:48:12 Richard W.M. Jones wrote: > Allows more sharing between the daemon and the inspection program. > --- > daemon/Makefile.am | 2 + > daemon/cleanups.c | 80 ++++++++++ > daemon/cleanups.h | 47 ++++++ > daemon/command.c | 436 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > daemon/command.h | 41 +++++ > daemon/daemon.h | 47 +----- > daemon/guestfsd.c | 392 ----------------------------------------------- > po/POTFILES | 2 + > 8 files changed, 611 insertions(+), 436 deletions(-) > create mode 100644 daemon/cleanups.c > create mode 100644 daemon/cleanups.h > create mode 100644 daemon/command.c > create mode 100644 daemon/command.h Mostly LGTM, just a couple of notes below. > +#ifndef GUESTFSD_CLEANUPS_H > +#define GUESTFSD_CLEANUPS_H > + > +/* Use by the CLEANUP_* macros. */ Can you please use the same comment as in guestfs-internal-frontend.h? > diff --git a/daemon/command.c b/daemon/command.c > new file mode 100644 > index 0000000..3e757aa > --- /dev/null > +++ b/daemon/command.c > @@ -0,0 +1,436 @@ > +/* libguestfs - the guestfsd daemon > + * Copyright (C) 2009-2015 Red Hat Inc. > + * > + * This program 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 of the License, or > + * (at your option) any later version. > + * > + * This program 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + */ > + > +#include <config.h> > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <unistd.h> > +#include <fcntl.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <sys/wait.h> > +#include <error.h> > +#include <errno.h> > + > +#include "ignore-value.h" > + > +#include "command.h" > +#include "cleanups.h" > + > +extern int verbose; > + > +extern const char *sysroot; > +extern size_t sysroot_len; > + > +#ifndef MAX > +# define MAX(a,b) ((a)>(b)?(a):(b)) > +#endif This is available in guestfs-internal-all.h -- shouldn't this new command.c include it as well? Should be useful also for the separate inspector tool, I guess.