[libvirt] [PATCH] build: silence recent syntax check violations

[Michal Privoznik]

On 30.03.2012 05:23, Eric Blake wrote:
> An upstream gnulib bug meant that some of our syntax checks
> weren't being run.  Fix up our offenders before we upgrade to
> a newer gnulib.
> 
> * src/util/virnetdevtap.c (virNetDevTapCreate): Use flags.
> * tests/lxcxml2xmltest.c (mymain): Strip useless ().
> ---
> 
> The gnulib bug was here:
> https://lists.gnu.org/archive/html/bug-gnulib/2012-03/msg00194.html
> 
> I tested this by temporarily using the fixed gnulib maint.mk.
> 
> Pushing under the trivial rule.  I can't call it a build breaker,
> because it won't break the build without a gnulib update.
> 
> I'm reluctant to update the .gnulib submodule this late in the
> game without some review, as we've had bad luck with a submodule
> update after the rc1 build in previous releases, so I'm saving
> that for another day.  Besides, I'm waiting for a review of a
> patch that fixes ssize_t for mingw, and it isn't worth a gnulib
> update without that fix.
> 
>  src/util/virnetdevtap.c |    7 +++++--
>  tests/lxcxml2xmltest.c  |    4 ++--
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
> index 0b3ac46..717b6ac 100644
> --- a/src/util/virnetdevtap.c
> +++ b/src/util/virnetdevtap.c
> @@ -129,12 +129,14 @@ virNetDevProbeVnetHdr(int tapfd)
>   */
>  int virNetDevTapCreate(char **ifname,
>                         int *tapfd,
> -                       unsigned int flags ATTRIBUTE_UNUSED)
> +                       unsigned int flags)
>  {
>      int fd;
>      struct ifreq ifr;
>      int ret = -1;
> 
> +    virCheckFlags(VIR_NETDEV_TAP_CREATE_VNET_HDR, -1);
> +

This is incomplete; networkStartNetworkVirtual() pass
VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE here. If a gnulib check is
causing fail, we should disable that check. Testing for flags at public
API is something that *have to* be done; omitting test at this low layer
and setting ATTRIBUTE_UNUSED is something that is intentional and
therefore is harmless.

Therefore we should either exclude src/util/* from checking, or drop
u_int flags completely here as they are unused after all. Okay, we can
also make virCheckFlags complete.

>      if ((fd = open("/dev/net/tun", O_RDWR)) < 0) {
>          virReportSystemError(errno, "%s",
>                               _("Unable to open /dev/net/tun, is tun module loaded?"));
> @@ -237,8 +239,9 @@ cleanup:
>  #else /* ! TUNSETIFF */
>  int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED,
>                         int *tapfd ATTRIBUTE_UNUSED,
> -                       unsigned int flags ATTRIBUTE_UNUSED)
> +                       unsigned int flags)
>  {
> +    virCheckFlags(0, -1);
>      virReportSystemError(ENOSYS, "%s",
>                           _("Unable to create TAP devices on this platform"));

However, this is even worse. Instead of throwing ENOSYS - the only
correct error here - we can throw some spurious error about unsupported
flags. This makes me lean towards excluding src/util/* from the check
causing trouble.

>      return -1;
> diff --git a/tests/lxcxml2xmltest.c b/tests/lxcxml2xmltest.c
> index 558bd01..6a87939 100644
> --- a/tests/lxcxml2xmltest.c
> +++ b/tests/lxcxml2xmltest.c
> @@ -99,7 +99,7 @@ mymain(void)
>      int ret = 0;
> 
>      if ((caps = testLXCCapsInit()) == NULL)
> -        return (EXIT_FAILURE);
> +        return EXIT_FAILURE;
> 
>  # define DO_TEST_FULL(name, is_different, inactive)                     \
>      do {                                                                \
> @@ -124,7 +124,7 @@ mymain(void)
> 
>      virCapabilitiesFree(caps);
> 
> -    return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
> +    return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
>  }
> 
>  VIRT_TEST_MAIN(mymain)

ACK to these two changes.

Michal