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

[Pino Toscano]

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.

Thanks,
-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20160122/bf5cdd50/attachment.sig>