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

[Richard W.M. Jones]

On Fri, Jan 22, 2016 at 11:03:28AM +0100, Pino Toscano wrote:
> 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.

I pushed this before reading your comments, but I'll push an updated
commit with these fixes.

Thanks,

Rich.


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW