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

Re: [libvirt] [Resend][PATCH 1/3] Add dnsmasq module



On 04/18/2010 09:53 PM, Satoru SATOH wrote:
> This patch adds the files implements dnsmasq (hostsfile) module.
> 
> Signed-off-by: Satoru SATOH <satoru satoh gmail com>

In addition to Jim's comments:

> 
> ---
>  src/util/dnsmasq.c |  326 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/util/dnsmasq.h |   59 ++++++++++
>  2 files changed, 385 insertions(+), 0 deletions(-)
>  create mode 100644 src/util/dnsmasq.c
>  create mode 100644 src/util/dnsmasq.h
> 
> diff --git a/src/util/dnsmasq.c b/src/util/dnsmasq.c
> new file mode 100644
> index 0000000..b46654e
> --- /dev/null
> +++ b/src/util/dnsmasq.c
> @@ -0,0 +1,326 @@
> +/*
> + * Copyright (C) 2010 Satoru SATOH <satoru satoh gmail com>
> + * Copyright (C) 2007-2009 Red Hat, Inc.

You did the right thing in preserving the Red Hat copyright from
src/iptables.c, since that was your template.  However, it makes
maintenance easier if you list the Red Hat line first, and extend its
copyright to 2010, since emacs copyright updating hooks only catch the
first copyright line.

> +
> +#ifdef HAVE_PATHS_H
> +#include <paths.h>
> +#endif

This fails 'make syntax-check' if you have cppi installed.  Indent the
include line:

#ifdef HAVE_PATHS_H
# include <paths.h>

> +static int
> +hostsfileWrite(const char *path, dnsmasqDhcpHost *hosts, int nhosts)
> +{
> +    char tmp[PATH_MAX];
> +    FILE *f;
> +    int istmp;

Make this 'bool istmp', for how you are using it.

> +
> +typedef struct
> +{
> +    int              nhosts;
> +    dnsmasqDhcpHost *hosts;
> +
> +    char   path[PATH_MAX];  /* Absolute path of dnsmasq's hostsfile. */
> +} dnsmasqHostsfile;

Jim didn't mention this use of PATH_MAX, but it is worth refactoring.
In particular, PATH_MAX can cause problems if we ever try to port to GNU
Hurd, where there is no limit (I know that the current libvirt code base
already has uses of PATH_MAX, but we shouldn't be adding more).

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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