[libvirt] [PATCH] virnetdev: fix some issues found by coverity and mingw builds

Commit e562a61a introduced new function to get/set interface state but there was misuse of ATTRIBUTE_NONNULL on non-pointer attributes and also we need to wrap that functions by #ifdef to not break mingw build. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/util/virnetdev.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++-- src/util/virnetdev.h | 12 +++---- 2 files changed, 96 insertions(+), 10 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 7a0a43d..d8a4867 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -671,12 +671,23 @@ int virNetDevSetIFFlag(const char *ifname, * * Returns 0 in case of success or -1 on error. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevSetOnline(const char *ifname, bool online) { return virNetDevSetIFFlag(ifname, IFF_UP, online); } +#else +int virNetDevSetOnline(const char *ifname, + bool online ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Cannot set interface flags on '%s'"), + ifname); + return -1; +} +#endif /** * virNetDevSetPromiscuous: @@ -689,11 +700,22 @@ int virNetDevSetOnline(const char *ifname, * * Returns 0 in case of success or -1 on error. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevSetPromiscuous(const char *ifname, bool promiscuous) { return virNetDevSetIFFlag(ifname, IFF_PROMISC, promiscuous); } +#else +int virNetDevSetPromiscuous(const char *ifname, + bool promiscuous ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Cannot set interface flags on '%s'"), + ifname); + return -1; +} +#endif /** * virNetDevSetRcvMulti: @@ -707,11 +729,22 @@ int virNetDevSetPromiscuous(const char *ifname, * * Returns 0 in case of success or -1 on error. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevSetRcvMulti(const char *ifname, bool receive) { return virNetDevSetIFFlag(ifname, IFF_MULTICAST, receive); } +#else +int virNetDevSetRcvMulti(const char *ifname, + bool receive ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Cannot set interface flags on '%s'"), + ifname); + return -1; +} +#endif /** * virNetDevSetRcvAllMulti: @@ -723,11 +756,22 @@ int virNetDevSetRcvMulti(const char *ifname, * * Returns 0 in case of success or -1 on error. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevSetRcvAllMulti(const char *ifname, bool receive) { return virNetDevSetIFFlag(ifname, IFF_ALLMULTI, receive); } +#else +int virNetDevSetRcvAllMulti(const char *ifname, + bool receive ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Cannot set interface flags on '%s'"), + ifname); + return -1; +} +#endif #if defined(SIOCGIFFLAGS) && defined(HAVE_STRUCT_IFREQ) @@ -776,11 +820,22 @@ int virNetDevGetIFFlag(const char *ifname, * * Returns 0 in case of success or an errno code in case of failure. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevGetOnline(const char *ifname, bool *online) { return virNetDevGetIFFlag(ifname, IFF_UP, online); } +#else +int virNetDevGetOnline(const char *ifname, + bool *online ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Cannot get interface flags on '%s'"), + ifname); + return -1; +} +#endif /** * virNetDevIsPromiscuous: @@ -792,11 +847,22 @@ int virNetDevGetOnline(const char *ifname, * * Returns 0 in case of success or an errno code in case of failure. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevGetPromiscuous(const char *ifname, - bool *promiscuous) + bool *promiscuous) { return virNetDevGetIFFlag(ifname, IFF_PROMISC, promiscuous); } +#else +int virNetDevGetPromiscuous(const char *ifname, + bool *promiscuous ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Cannot get interface flags on '%s'"), + ifname); + return -1; +} +#endif /** * virNetDevIsRcvMulti: @@ -808,11 +874,22 @@ int virNetDevGetPromiscuous(const char *ifname, * * Returns 0 in case of success or -1 on error. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevGetRcvMulti(const char *ifname, - bool *receive) + bool *receive) { return virNetDevGetIFFlag(ifname, IFF_MULTICAST, receive); } +#else +int virNetDevGetRcvMulti(const char *ifname, + bool *receive ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Cannot get interface flags on '%s'"), + ifname); + return -1; +} +#endif /** * virNetDevIsRcvAllMulti: @@ -824,11 +901,22 @@ int virNetDevGetRcvMulti(const char *ifname, * * Returns 0 in case of success or -1 on error. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevGetRcvAllMulti(const char *ifname, - bool *receive) + bool *receive) { return virNetDevGetIFFlag(ifname, IFF_ALLMULTI, receive); } +#else +int virNetDevGetRcvAllMulti(const char *ifname, + bool *receive ATTRIBUTE_UNUSED) +{ + virReportSystemError(ENOSYS, + _("Cannot get interface flags on '%s'"), + ifname); + return -1; +} +#endif /** diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 8d03459..1001e43 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -201,25 +201,23 @@ int virNetDevDelMulti(const char *ifname, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevSetIFFlag(const char *ifname, int flag, bool val) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevGetIFFlag(const char *ifname, int flag, bool *val) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; int virNetDevSetPromiscuous(const char *ifname, bool promiscuous) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevGetPromiscuous(const char *ifname, bool *promiscuous) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevSetRcvMulti(const char *ifname, bool receive) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevGetRcvMulti(const char *ifname, bool *receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevSetRcvAllMulti(const char *ifname, bool receive) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevGetRcvAllMulti(const char *ifname, bool *receive) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; #endif /* __VIR_NETDEV_H__ */ -- 2.0.5

On 02/02/2015 11:31 AM, Pavel Hrdina wrote:
Commit e562a61a introduced new function to get/set interface state but there was misuse of ATTRIBUTE_NONNULL on non-pointer attributes and also we need to wrap that functions by #ifdef to not break mingw build.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
+++ b/src/util/virnetdev.c @@ -671,12 +671,23 @@ int virNetDevSetIFFlag(const char *ifname, * * Returns 0 in case of success or -1 on error. */ +#if defined(SIOCSIFFLAGS) && defined(HAVE_STRUCT_IFREQ) int virNetDevSetOnline(const char *ifname, bool online) {
return virNetDevSetIFFlag(ifname, IFF_UP, online); }
This feels like it should be safe, except maybe for the fact that it uses IFF_UP. Maybe we should wrap that under VIR_IFF_UP (defined to IFF_UP when the #ifdefs are right, and to 0 otherwise), so that _only_ virNetDevSetIFFlag() has to have a counterpart definition, instead of repeating lots of #ifdefs on all the callers.
+++ b/src/util/virnetdev.h @@ -201,25 +201,23 @@ int virNetDevDelMulti(const char *ifname, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
int virNetDevSetIFFlag(const char *ifname, int flag, bool val) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_RETURN_CHECK; + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
Ack to the changes in this file, although I still think the virnetdev.c changes can be made shorter. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Pavel Hrdina