[libvirt] [PATCH 0/2] Fix the non-linux platform build after recent VIR_AUTOPTR patches

There were some complaints about an unused variable (patch 1) and unused static functions which are defined by the VIR_DEFINE_AUTOPTR_FUNC macro (patch 2). Successful Travis build: https://travis-ci.org/eskultety/libvirt/builds/413187470 MinGW wasn't affected by the recent VIR_AUTOPTR series: https://ci.centos.org/view/libvirt/job/libvirt-master-build-mingw64/330/ *NOT* pushing under the build breaker in case someone wants to have a look. Erik Skultety (2): util: virnetdevopenvswitch: Drop an unused variable @ovs_timeout Fix the build on non-linux platforms after VIR_AUTOPTR related changes src/util/virnetdev.c | 3 +-- src/util/virnetdevopenvswitch.c | 1 - src/util/virnetlink.c | 6 ++---- 3 files changed, 3 insertions(+), 7 deletions(-) -- 2.14.4

Technically, it was never used ever since commit @f4d06ca8fd9 introduced it, but the fact that we called VIR_FREE on it was enough for Clang to never complain about it. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- CC'ing Boris, as he allowed specifying timeouts for openvswitch calls, however, this particular one hasn't been in use ever. src/util/virnetdevopenvswitch.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 69ae04ee5a..9a9435f8af 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -473,7 +473,6 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, size_t ntokens = 0; int status; int ret = -1; - VIR_AUTOFREE(char *) ovs_timeout = NULL; /* Openvswitch vhostuser path are hardcoded to * /<runstatedir>/openvswitch/<ifname> -- 2.14.4

On Wed, Aug 08, 2018 at 08:23:41AM +0200, Erik Skultety wrote:
Technically, it was never used ever since commit @f4d06ca8fd9 introduced it, but the fact that we called VIR_FREE on it was enough for Clang to never complain about it.
Signed-off-by: Erik Skultety <eskultet@redhat.com> ---
CC'ing Boris, as he allowed specifying timeouts for openvswitch calls, however, this particular one hasn't been in use ever.
Looks like an oversight by Michal when he made adjustments before pushing the series: https://www.redhat.com/archives/libvir-list/2017-February/msg00407.html The timeout addition is handled by virNetDevOpenvswitchAddTimeout, this is harmless.
src/util/virnetdevopenvswitch.c | 1 - 1 file changed, 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On 08/08/2018 08:23 AM, Erik Skultety wrote:
Technically, it was never used ever since commit @f4d06ca8fd9 introduced it, but the fact that we called VIR_FREE on it was enough for Clang to never complain about it.
Signed-off-by: Erik Skultety <eskultet@redhat.com> ---
CC'ing Boris, as he allowed specifying timeouts for openvswitch calls, however, this particular one hasn't been in use ever.
src/util/virnetdevopenvswitch.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 69ae04ee5a..9a9435f8af 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -473,7 +473,6 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path, size_t ntokens = 0; int status; int ret = -1; - VIR_AUTOFREE(char *) ovs_timeout = NULL;
/* Openvswitch vhostuser path are hardcoded to * /<runstatedir>/openvswitch/<ifname> -- 2.14.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Even so it is pushed already the removal is correct. I did not catch this leftover after Michal rework. -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

Commits 7b706f33ac and 4acb7887e4 introduced some compound type *Free wrappers in order to use them with VIR_DEFINE_AUTOPTR_FUNC. However, since those were not used in the code right away, Clang complained about unused functions (static ones that are defined by the macro above). This patch puts the defined functions in use. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/virnetdev.c | 3 +-- src/util/virnetlink.c | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 9eca786c95..8eac419725 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2833,7 +2833,7 @@ static int virNetDevGetMcastList(const char *ifname, char *buf = NULL; char *next = NULL; int ret = -1, len; - virNetDevMcastEntryPtr entry = NULL; + VIR_AUTOPTR(virNetDevMcastEntry) entry = NULL; mcast->entries = NULL; mcast->nentries = 0; @@ -2867,7 +2867,6 @@ static int virNetDevGetMcastList(const char *ifname, ret = 0; cleanup: VIR_FREE(buf); - VIR_FREE(entry); return ret; } diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 2702c12d24..162efe6f99 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -305,7 +305,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg, .nl_groups = 0, }; struct pollfd fds[1]; - virNetlinkHandle *nlhandle = NULL; + VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; int len = 0; memset(fds, 0, sizeof(fds)); @@ -333,7 +333,6 @@ int virNetlinkCommand(struct nl_msg *nl_msg, *respbuflen = 0; } - virNetlinkFree(nlhandle); return ret; } @@ -355,7 +354,7 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, .nl_pid = dst_pid, .nl_groups = 0, }; - virNetlinkHandle *nlhandle = NULL; + VIR_AUTOPTR(virNetlinkHandle) nlhandle = NULL; if (!(nlhandle = virNetlinkSendRequest(nl_msg, src_pid, nladdr, protocol, groups))) @@ -382,7 +381,6 @@ virNetlinkDumpCommand(struct nl_msg *nl_msg, cleanup: VIR_FREE(resp); - virNetlinkFree(nlhandle); return ret; } -- 2.14.4

On Wed, Aug 08, 2018 at 08:23:42AM +0200, Erik Skultety wrote:
Commits 7b706f33ac and 4acb7887e4 introduced some compound type *Free wrappers in order to use them with VIR_DEFINE_AUTOPTR_FUNC. However, since those were not used in the code right away, Clang complained about unused functions (static ones that are defined by the macro above). This patch puts the defined functions in use.
If you keep the series bisectable even with clang with warnings turned on, you can GET a cookie!
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/virnetdev.c | 3 +-- src/util/virnetlink.c | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-)
This fixes the build for me with CLang 5.0.1 Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Wed, Aug 08, 2018 at 09:16:20AM +0200, Ján Tomko wrote:
On Wed, Aug 08, 2018 at 08:23:42AM +0200, Erik Skultety wrote:
Commits 7b706f33ac and 4acb7887e4 introduced some compound type *Free wrappers in order to use them with VIR_DEFINE_AUTOPTR_FUNC. However, since those were not used in the code right away, Clang complained about unused functions (static ones that are defined by the macro above). This patch puts the defined functions in use.
If you keep the series bisectable even with clang with warnings turned on, you can GET a cookie!
Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/util/virnetdev.c | 3 +-- src/util/virnetlink.c | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-)
This fixes the build for me with CLang 5.0.1
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pushed, thanks. Erik
participants (3)
-
Boris Fiuczynski
-
Erik Skultety
-
Ján Tomko