[libvirt] [PATCH 2/2] util: virNetDevBandwidthPlug: Drop ATTRIBUTE_UNUSED(4)

[Jiri Denemark]

On Fri, Nov 22, 2019 at 17:09:00 +0100, Erik Skultety wrote:
> Since we know for sure that the @bandwidth parameter is properly handled
> in networkCheckBandwidth (@ifaceBand), it cannot ever be NULL, but
> coverity doesn't see this fact. In order to prevent coverity from
> reporting a false positive here, drop ATTRIBUTE_UNUSED for this specific

s/ATTRIBUTE_UNUSED/ATTRIBUTE_NONNULL/ both in the subject and the commit
message, however...

> argument - virNetDevBandwidthPlug is also called from a single place only
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  src/util/virnetdevbandwidth.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
> index 19323c5ed2..59d7513286 100644
> --- a/src/util/virnetdevbandwidth.h
> +++ b/src/util/virnetdevbandwidth.h
> @@ -55,8 +55,7 @@ int virNetDevBandwidthPlug(const char *brname,
>                             const virMacAddr *ifmac_ptr,
>                             virNetDevBandwidthPtr bandwidth,
>                             unsigned int id)
> -    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
> -    G_GNUC_WARN_UNUSED_RESULT;
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT;
>  
>  int virNetDevBandwidthUnplug(const char *brname,
>                               unsigned int id)

I don't think this is a good solution. From the point of view of this
function ATTRIBUTE_NONNULL is correct for all three parameters and
removing it for one of them does not make sense.

Since we just want to silence coverity's false positive, we could use
sa_assert in the right place to teach coverity virNetDevBandwidthPlug is
not ever called with bandwidth == NULL:

    diff --git i/src/network/bridge_driver.c w/src/network/bridge_driver.c
    index 9b12b98d88..52c8ba4c73 100644
    --- i/src/network/bridge_driver.c
    +++ w/src/network/bridge_driver.c
    @@ -5263,6 +5263,8 @@ networkPlugBandwidth(virNetworkObjPtr obj,
             /* no QoS needs to be set; claim success */
             return 0;
     
    +    sa_assert(ifaceBand);
    +
         virMacAddrFormat(mac, ifmac);
     
         if (networkPlugBandwidthImpl(obj, mac, ifaceBand, class_id, new_rate) < 0)

Jirka