[libvirt] PATCH: Fix passing of ifname to QEMU

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:
> When using the type='ethernet'  network device configuration for a guest
> we pass a script, and optional interface name to QEMU. If ifname is
> omitted, then QEMU allocates one itself. The problem was we were passing
> an ifname of '(null)' by mistake. This patch corrects that problem and
> adds a test for it.
>
> Daniel
>
> diff -r b6a065030fa6 src/qemu_conf.c
> --- a/src/qemu_conf.c	Fri Jan 30 12:28:00 2009 +0000
> +++ b/src/qemu_conf.c	Fri Jan 30 13:07:11 2009 +0000
> @@ -1147,11 +1147,18 @@ int qemudBuildCommandLine(virConnectPtr
>              case VIR_DOMAIN_NET_TYPE_ETHERNET:
>                  {
>                      char arg[PATH_MAX];
> -                    if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d",
> -                                 net->ifname,
> -                                 net->data.ethernet.script,
> -                                 vlan) >= (PATH_MAX-1))
> -                        goto error;


> +                    if (net->ifname) {
> +                        if (snprintf(arg, PATH_MAX-1, "tap,ifname=%s,script=%s,vlan=%d",
> +                                     net->ifname,
> +                                     net->data.ethernet.script,
> +                                     vlan) >= (PATH_MAX-1))
> +                            goto error;
> +                    } else {
> +                        if (snprintf(arg, PATH_MAX-1, "tap,script=%s,vlan=%d",
> +                                     net->data.ethernet.script,
> +                                     vlan) >= (PATH_MAX-1))
> +                            goto error;
> +                    }


Along the way, it'd be nice to use "sizeof(arg)-1" in place of
PATH_MAX-1.

That format string and arg list are long enough
that it'd be good not to duplicate them.
How about this instead?

             char if_buf[PATH_MAX];
             if_buf[0] = 0;
             if (net->ifname
                 && (snprintf(if_buf, sizeof(if_buf), ",script=%s", ifname)
                     >= sizeof(if_buf)-1))
                 goto error;

             if (snprintf(arg, sizeof(arg), "tap%s,script=%s,vlan=%d",
                          if_buf,
                          net->data.ethernet.script,
                          vlan) >= sizeof(arg)-1))
                 goto error;

+1 for the test
ACK either way.