[libvirt] [PATCH 0/3] Misc RPC fixes
Misc fixes pointed out by John after coverity had a fit :-) Daniel P. Berrangé (3): tests: validate private data / pre / post exec hooks for RPC APIs rpc: assume private data callbacks are always non-NULL rpc: fix non-NULL annotations when GNUTLS is disabled src/rpc/virnetserverclient.c | 47 +++++++------- src/rpc/virnetserverclient.h | 4 ++ .../virnetdaemondata/input-data-admin-nomdns.json | 12 ++-- .../input-data-admin-server-names.json | 12 ++-- .../virnetdaemondata/input-data-anon-clients.json | 6 +- .../input-data-client-auth-pending-failure.json | 3 +- .../input-data-client-auth-pending.json | 6 +- tests/virnetdaemondata/input-data-client-ids.json | 6 +- .../input-data-client-timestamp.json | 6 +- .../input-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/input-data-initial.json | 6 +- .../input-data-no-keepalive-required.json | 12 ++-- .../virnetdaemondata/output-data-admin-nomdns.json | 12 ++-- .../output-data-admin-server-names.json | 12 ++-- .../virnetdaemondata/output-data-anon-clients.json | 6 +- .../output-data-client-auth-pending.json | 6 +- tests/virnetdaemondata/output-data-client-ids.json | 6 +- .../output-data-client-timestamp.json | 6 +- .../output-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/output-data-initial.json | 6 +- .../output-data-no-keepalive-required.json | 12 ++-- tests/virnetdaemontest.c | 71 +++++++++++++++++++--- tests/virnetserverclienttest.c | 22 ++++++- 23 files changed, 208 insertions(+), 83 deletions(-) -- 2.14.3
Validate that the virNetServer(Client) RPC APIs are processing the private data callbacks correctly by passing in non-NULL pointers. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .../virnetdaemondata/input-data-admin-nomdns.json | 12 ++-- .../input-data-admin-server-names.json | 12 ++-- .../virnetdaemondata/input-data-anon-clients.json | 6 +- .../input-data-client-auth-pending-failure.json | 3 +- .../input-data-client-auth-pending.json | 6 +- tests/virnetdaemondata/input-data-client-ids.json | 6 +- .../input-data-client-timestamp.json | 6 +- .../input-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/input-data-initial.json | 6 +- .../input-data-no-keepalive-required.json | 12 ++-- .../virnetdaemondata/output-data-admin-nomdns.json | 12 ++-- .../output-data-admin-server-names.json | 12 ++-- .../virnetdaemondata/output-data-anon-clients.json | 6 +- .../output-data-client-auth-pending.json | 6 +- tests/virnetdaemondata/output-data-client-ids.json | 6 +- .../output-data-client-timestamp.json | 6 +- .../output-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/output-data-initial.json | 6 +- .../output-data-no-keepalive-required.json | 12 ++-- tests/virnetdaemontest.c | 71 +++++++++++++++++++--- tests/virnetserverclienttest.c | 22 ++++++- 21 files changed, 183 insertions(+), 57 deletions(-) diff --git a/tests/virnetdaemondata/input-data-admin-nomdns.json b/tests/virnetdaemondata/input-data-admin-nomdns.json index 449bcc9b25..985ea4b434 100644 --- a/tests/virnetdaemondata/input-data-admin-nomdns.json +++ b/tests/virnetdaemondata/input-data-admin-nomdns.json @@ -46,7 +46,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -57,7 +58,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] }, @@ -107,7 +109,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -118,7 +121,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/input-data-admin-server-names.json b/tests/virnetdaemondata/input-data-admin-server-names.json index 3afe16fad0..99515c5f12 100644 --- a/tests/virnetdaemondata/input-data-admin-server-names.json +++ b/tests/virnetdaemondata/input-data-admin-server-names.json @@ -48,7 +48,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -59,7 +60,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] }, @@ -110,7 +112,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -121,7 +124,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/input-data-anon-clients.json b/tests/virnetdaemondata/input-data-anon-clients.json index 8a51ff53d6..8058fe0a43 100644 --- a/tests/virnetdaemondata/input-data-anon-clients.json +++ b/tests/virnetdaemondata/input-data-anon-clients.json @@ -45,7 +45,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -56,7 +57,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/input-data-client-auth-pending-failure.json b/tests/virnetdaemondata/input-data-client-auth-pending-failure.json index 7d4ef6438d..9c33d92e08 100644 --- a/tests/virnetdaemondata/input-data-client-auth-pending-failure.json +++ b/tests/virnetdaemondata/input-data-client-auth-pending-failure.json @@ -36,7 +36,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, ] } diff --git a/tests/virnetdaemondata/input-data-client-auth-pending.json b/tests/virnetdaemondata/input-data-client-auth-pending.json index ae85253b53..56dd2d85d6 100644 --- a/tests/virnetdaemondata/input-data-client-auth-pending.json +++ b/tests/virnetdaemondata/input-data-client-auth-pending.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 3, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/input-data-client-ids.json b/tests/virnetdaemondata/input-data-client-ids.json index 7caa2ccd57..c94a538b13 100644 --- a/tests/virnetdaemondata/input-data-client-ids.json +++ b/tests/virnetdaemondata/input-data-client-ids.json @@ -48,7 +48,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 3, @@ -60,7 +61,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/input-data-client-timestamp.json b/tests/virnetdaemondata/input-data-client-timestamp.json index d069997a14..ee2b2357be 100644 --- a/tests/virnetdaemondata/input-data-client-timestamp.json +++ b/tests/virnetdaemondata/input-data-client-timestamp.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/input-data-initial-nomdns.json b/tests/virnetdaemondata/input-data-initial-nomdns.json index 02bb427487..6cf70d96e8 100644 --- a/tests/virnetdaemondata/input-data-initial-nomdns.json +++ b/tests/virnetdaemondata/input-data-initial-nomdns.json @@ -44,7 +44,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -55,7 +56,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/input-data-initial.json b/tests/virnetdaemondata/input-data-initial.json index 795622535b..5bb09ed36b 100644 --- a/tests/virnetdaemondata/input-data-initial.json +++ b/tests/virnetdaemondata/input-data-initial.json @@ -45,7 +45,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -56,7 +57,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/input-data-no-keepalive-required.json b/tests/virnetdaemondata/input-data-no-keepalive-required.json index df282edd6e..1ed9ac2c7c 100644 --- a/tests/virnetdaemondata/input-data-no-keepalive-required.json +++ b/tests/virnetdaemondata/input-data-no-keepalive-required.json @@ -45,7 +45,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -56,7 +57,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] }, @@ -105,7 +107,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "auth": 2, @@ -116,7 +119,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-admin-nomdns.json b/tests/virnetdaemondata/output-data-admin-nomdns.json index d6d02163e2..04cb5e6bb3 100644 --- a/tests/virnetdaemondata/output-data-admin-nomdns.json +++ b/tests/virnetdaemondata/output-data-admin-nomdns.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] }, @@ -115,7 +117,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -128,7 +131,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-admin-server-names.json b/tests/virnetdaemondata/output-data-admin-server-names.json index d6d02163e2..04cb5e6bb3 100644 --- a/tests/virnetdaemondata/output-data-admin-server-names.json +++ b/tests/virnetdaemondata/output-data-admin-server-names.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] }, @@ -115,7 +117,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -128,7 +131,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-anon-clients.json b/tests/virnetdaemondata/output-data-anon-clients.json index cb6005bfe6..49fe89be48 100644 --- a/tests/virnetdaemondata/output-data-anon-clients.json +++ b/tests/virnetdaemondata/output-data-anon-clients.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-client-auth-pending.json b/tests/virnetdaemondata/output-data-client-auth-pending.json index 5720ea9b71..0675404e6c 100644 --- a/tests/virnetdaemondata/output-data-client-auth-pending.json +++ b/tests/virnetdaemondata/output-data-client-auth-pending.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 3, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-client-ids.json b/tests/virnetdaemondata/output-data-client-ids.json index 2b1663d4f8..90c3383a93 100644 --- a/tests/virnetdaemondata/output-data-client-ids.json +++ b/tests/virnetdaemondata/output-data-client-ids.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 3, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-client-timestamp.json b/tests/virnetdaemondata/output-data-client-timestamp.json index e8c8e9a991..9cfb069793 100644 --- a/tests/virnetdaemondata/output-data-client-timestamp.json +++ b/tests/virnetdaemondata/output-data-client-timestamp.json @@ -50,7 +50,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -64,7 +65,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-initial-nomdns.json b/tests/virnetdaemondata/output-data-initial-nomdns.json index 167aef8d29..916297c59d 100644 --- a/tests/virnetdaemondata/output-data-initial-nomdns.json +++ b/tests/virnetdaemondata/output-data-initial-nomdns.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-initial.json b/tests/virnetdaemondata/output-data-initial.json index 3281e868d7..5ed03472d6 100644 --- a/tests/virnetdaemondata/output-data-initial.json +++ b/tests/virnetdaemondata/output-data-initial.json @@ -50,7 +50,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -63,7 +64,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemondata/output-data-no-keepalive-required.json b/tests/virnetdaemondata/output-data-no-keepalive-required.json index d6d02163e2..04cb5e6bb3 100644 --- a/tests/virnetdaemondata/output-data-no-keepalive-required.json +++ b/tests/virnetdaemondata/output-data-no-keepalive-required.json @@ -49,7 +49,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -62,7 +63,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] }, @@ -115,7 +117,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 }, { "id": 2, @@ -128,7 +131,8 @@ "errfd": -1, "pid": -1, "isClient": true - } + }, + "privateData": 1729 } ] } diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 435513d314..8cda06b197 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -27,6 +27,54 @@ #define VIR_FROM_THIS VIR_FROM_RPC #if defined(HAVE_SOCKETPAIR) && defined(WITH_YAJL) +struct testClientPriv { + int magic; +}; + +static void *testClientNew(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + struct testClientPriv *priv; + + if (VIR_ALLOC(priv) < 0) + return NULL; + + priv->magic = 1729; + + return priv; +} + +static virJSONValuePtr testClientPreExec(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *data) +{ + struct testClientPriv *priv = data; + + return virJSONValueNewNumberInt(priv->magic); +} + + +static void *testClientNewPostExec(virNetServerClientPtr client, + virJSONValuePtr object, + void *opaque) +{ + int magic; + + if (virJSONValueGetNumberInt(object, &magic) < 0) { + return NULL; + } + + if (magic != 1729) { + return NULL; + } + + return testClientNew(client, opaque); +} + +static void testClientFree(void *opaque) +{ + VIR_FREE(opaque); +} + static virNetServerPtr testCreateServer(const char *server_name, const char *host, int family) { @@ -53,9 +101,9 @@ testCreateServer(const char *server_name, const char *host, int family) 10, 50, 5, 100, 10, 120, 5, mdns_group, - NULL, - NULL, - NULL, + testClientNew, + testClientPreExec, + testClientFree, NULL))) goto error; @@ -101,7 +149,10 @@ testCreateServer(const char *server_name, const char *host, int family) # ifdef WITH_GNUTLS NULL, # endif - NULL, NULL, NULL, NULL))) + testClientNew, + testClientPreExec, + testClientFree, + NULL))) goto error; if (!(cln2 = virNetServerClientNew(virNetServerNextClientID(srv), @@ -112,7 +163,10 @@ testCreateServer(const char *server_name, const char *host, int family) # ifdef WITH_GNUTLS NULL, # endif - NULL, NULL, NULL, NULL))) + testClientNew, + testClientPreExec, + testClientFree, + NULL))) goto error; if (virNetServerAddClient(srv, cln1) < 0) @@ -206,8 +260,11 @@ testNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, if (STREQ(data->serverNames[i], name)) { return virNetServerNewPostExecRestart(object, name, - NULL, NULL, NULL, - NULL, NULL); + NULL, + testClientNewPostExec, + testClientPreExec, + testClientFree, + NULL); } } diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c index 96b69b3e45..351ba9f4c7 100644 --- a/tests/virnetserverclienttest.c +++ b/tests/virnetserverclienttest.c @@ -27,6 +27,23 @@ #define VIR_FROM_THIS VIR_FROM_RPC #ifdef HAVE_SOCKETPAIR + +static void *testClientNew(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + char *dummy; + + if (VIR_ALLOC(dummy) < 0) + return NULL; + + return dummy; +} + +static void testClientFree(void *opaque) +{ + VIR_FREE(opaque); +} + static int testIdentity(const void *opaque ATTRIBUTE_UNUSED) { int sv[2]; @@ -56,7 +73,10 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED) # ifdef WITH_GNUTLS NULL, # endif - NULL, NULL, NULL, NULL))) { + testClientNew, + NULL, + testClientFree, + NULL))) { virDispatchError(NULL); goto cleanup; } -- 2.14.3
On 02/01/2018 10:35 AM, Daniel P. Berrangé wrote:
Validate that the virNetServer(Client) RPC APIs are processing the private data callbacks correctly by passing in non-NULL pointers.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- .../virnetdaemondata/input-data-admin-nomdns.json | 12 ++-- .../input-data-admin-server-names.json | 12 ++-- .../virnetdaemondata/input-data-anon-clients.json | 6 +- .../input-data-client-auth-pending-failure.json | 3 +- .../input-data-client-auth-pending.json | 6 +- tests/virnetdaemondata/input-data-client-ids.json | 6 +- .../input-data-client-timestamp.json | 6 +- .../input-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/input-data-initial.json | 6 +- .../input-data-no-keepalive-required.json | 12 ++-- .../virnetdaemondata/output-data-admin-nomdns.json | 12 ++-- .../output-data-admin-server-names.json | 12 ++-- .../virnetdaemondata/output-data-anon-clients.json | 6 +- .../output-data-client-auth-pending.json | 6 +- tests/virnetdaemondata/output-data-client-ids.json | 6 +- .../output-data-client-timestamp.json | 6 +- .../output-data-initial-nomdns.json | 6 +- tests/virnetdaemondata/output-data-initial.json | 6 +- .../output-data-no-keepalive-required.json | 12 ++-- tests/virnetdaemontest.c | 71 +++++++++++++++++++--- tests/virnetserverclienttest.c | 22 ++++++- 21 files changed, 183 insertions(+), 57 deletions(-)
[...]
--- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -27,6 +27,54 @@ #define VIR_FROM_THIS VIR_FROM_RPC
#if defined(HAVE_SOCKETPAIR) && defined(WITH_YAJL) +struct testClientPriv { + int magic; +}; + +static void *testClientNew(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED)
These are stylistically like the rest... static void ** testClientNew(...)
+{ + struct testClientPriv *priv; + + if (VIR_ALLOC(priv) < 0) + return NULL; + + priv->magic = 1729; + + return priv; +} + +static virJSONValuePtr testClientPreExec(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *data) +{ + struct testClientPriv *priv = data; + + return virJSONValueNewNumberInt(priv->magic); +} + + +static void *testClientNewPostExec(virNetServerClientPtr client, + virJSONValuePtr object, + void *opaque) +{ + int magic; + + if (virJSONValueGetNumberInt(object, &magic) < 0) { + return NULL; + } + + if (magic != 1729) { + return NULL; + }
These don't pass curly bracket syntax-check...
+ + return testClientNew(client, opaque); +} + +static void testClientFree(void *opaque) +{ + VIR_FREE(opaque); +} + static virNetServerPtr testCreateServer(const char *server_name, const char *host, int family) { @@ -53,9 +101,9 @@ testCreateServer(const char *server_name, const char *host, int family) 10, 50, 5, 100, 10, 120, 5, mdns_group, - NULL, - NULL, - NULL, + testClientNew, + testClientPreExec, + testClientFree, NULL))) goto error;
@@ -101,7 +149,10 @@ testCreateServer(const char *server_name, const char *host, int family) # ifdef WITH_GNUTLS NULL, # endif - NULL, NULL, NULL, NULL))) + testClientNew, + testClientPreExec, + testClientFree, + NULL))) goto error;
if (!(cln2 = virNetServerClientNew(virNetServerNextClientID(srv), @@ -112,7 +163,10 @@ testCreateServer(const char *server_name, const char *host, int family) # ifdef WITH_GNUTLS NULL, # endif - NULL, NULL, NULL, NULL))) + testClientNew, + testClientPreExec, + testClientFree, + NULL))) goto error;
if (virNetServerAddClient(srv, cln1) < 0) @@ -206,8 +260,11 @@ testNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, if (STREQ(data->serverNames[i], name)) { return virNetServerNewPostExecRestart(object, name, - NULL, NULL, NULL, - NULL, NULL); + NULL, + testClientNewPostExec, + testClientPreExec, + testClientFree, + NULL);
virnetdaemontest.c: In function 'testNewServerPostExecRestart': virnetdaemontest.c:261:20: error: null argument where non-null required (argument 3) [-Werror=nonnull] return virNetServerNewPostExecRestart(object, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors I added testClient{New|Free} and things were happy.
} }
diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c index 96b69b3e45..351ba9f4c7 100644 --- a/tests/virnetserverclienttest.c +++ b/tests/virnetserverclienttest.c @@ -27,6 +27,23 @@ #define VIR_FROM_THIS VIR_FROM_RPC
#ifdef HAVE_SOCKETPAIR + +static void *testClientNew(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED)
Stylistically same here... With nits fixed, Reviewed-by: John Ferlan <jferlan@redhat.com> John
+{ + char *dummy; + + if (VIR_ALLOC(dummy) < 0) + return NULL; + + return dummy; +} + +static void testClientFree(void *opaque) +{ + VIR_FREE(opaque); +} + static int testIdentity(const void *opaque ATTRIBUTE_UNUSED) { int sv[2]; @@ -56,7 +73,10 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED) # ifdef WITH_GNUTLS NULL, # endif - NULL, NULL, NULL, NULL))) { + testClientNew, + NULL, + testClientFree, + NULL))) { virDispatchError(NULL); goto cleanup; }
Since we annotate the APIs are having non-NULL parameters, we can remove the checks for NULL in the code too. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetserverclient.c | 47 ++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index d81a3892ff..388514946b 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -466,14 +466,12 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, now))) return NULL; - if (privNew) { - if (!(client->privateData = privNew(client, privOpaque))) { - virObjectUnref(client); - return NULL; - } - client->privateDataFreeFunc = privFree; - client->privateDataPreExecRestart = privPreExecRestart; + if (!(client->privateData = privNew(client, privOpaque))) { + virObjectUnref(client); + return NULL; } + client->privateDataFreeFunc = privFree; + client->privateDataPreExecRestart = privPreExecRestart; return client; } @@ -580,18 +578,18 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, } virObjectUnref(sock); - if (privNew) { - if (!(child = virJSONValueObjectGet(object, "privateData"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing privateData field in JSON state document")); - goto error; - } - if (!(client->privateData = privNew(client, child, privOpaque))) - goto error; - client->privateDataFreeFunc = privFree; - client->privateDataPreExecRestart = privPreExecRestart; + if (!(child = virJSONValueObjectGet(object, "privateData"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing privateData field in JSON state document")); + goto error; } + if (!(client->privateData = privNew(client, child, privOpaque))) + goto error; + + client->privateDataFreeFunc = privFree; + client->privateDataPreExecRestart = privPreExecRestart; + return client; @@ -637,14 +635,12 @@ virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client) goto error; } - if (client->privateData && client->privateDataPreExecRestart) { - if (!(child = client->privateDataPreExecRestart(client, client->privateData))) - goto error; + if (!(child = client->privateDataPreExecRestart(client, client->privateData))) + goto error; - if (virJSONValueObjectAppend(object, "privateData", child) < 0) { - virJSONValueFree(child); - goto error; - } + if (virJSONValueObjectAppend(object, "privateData", child) < 0) { + virJSONValueFree(child); + goto error; } virObjectUnlock(client); @@ -989,8 +985,7 @@ void virNetServerClientDispose(void *obj) PROBE(RPC_SERVER_CLIENT_DISPOSE, "client=%p", client); - if (client->privateData && - client->privateDataFreeFunc) + if (client->privateData) client->privateDataFreeFunc(client->privateData); virObjectUnref(client->identity); -- 2.14.3
On 02/01/2018 10:35 AM, Daniel P. Berrangé wrote:
Since we annotate the APIs are having non-NULL parameters, we can remove the checks for NULL in the code too.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetserverclient.c | 47 ++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 26 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index d81a3892ff..388514946b 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -466,14 +466,12 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, now))) return NULL;
- if (privNew) { - if (!(client->privateData = privNew(client, privOpaque))) { - virObjectUnref(client); - return NULL; - } - client->privateDataFreeFunc = privFree; - client->privateDataPreExecRestart = privPreExecRestart; + if (!(client->privateData = privNew(client, privOpaque))) { + virObjectUnref(client); + return NULL; } + client->privateDataFreeFunc = privFree; + client->privateDataPreExecRestart = privPreExecRestart;
return client; } @@ -580,18 +578,18 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, } virObjectUnref(sock);
- if (privNew) { - if (!(child = virJSONValueObjectGet(object, "privateData"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Missing privateData field in JSON state document")); - goto error; - } - if (!(client->privateData = privNew(client, child, privOpaque))) - goto error; - client->privateDataFreeFunc = privFree; - client->privateDataPreExecRestart = privPreExecRestart; + if (!(child = virJSONValueObjectGet(object, "privateData"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing privateData field in JSON state document")); + goto error; }
+ if (!(client->privateData = privNew(client, child, privOpaque))) + goto error; + + client->privateDataFreeFunc = privFree; + client->privateDataPreExecRestart = privPreExecRestart; +
return client;
@@ -637,14 +635,12 @@ virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client) goto error; }
- if (client->privateData && client->privateDataPreExecRestart) { - if (!(child = client->privateDataPreExecRestart(client, client->privateData))) - goto error; + if (!(child = client->privateDataPreExecRestart(client, client->privateData))) + goto error;
- if (virJSONValueObjectAppend(object, "privateData", child) < 0) { - virJSONValueFree(child); - goto error; - } + if (virJSONValueObjectAppend(object, "privateData", child) < 0) { + virJSONValueFree(child); + goto error; }
virObjectUnlock(client); @@ -989,8 +985,7 @@ void virNetServerClientDispose(void *obj) PROBE(RPC_SERVER_CLIENT_DISPOSE, "client=%p", client);
- if (client->privateData && - client->privateDataFreeFunc) + if (client->privateData) client->privateDataFreeFunc(client->privateData);
virObjectUnref(client->identity);
The position of various parameters changes depending on the WITH_GNUTLS macro. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetserverclient.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 4a0d3cc25e..1a939ad4e1 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -73,7 +73,11 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, void *privOpaque) +# ifdef WITH_GNUTLS ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(9); +# else + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(6) ATTRIBUTE_NONNULL(8); +# endif virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, virJSONValuePtr object, -- 2.14.3
On 02/01/2018 10:35 AM, Daniel P. Berrangé wrote:
The position of various parameters changes depending on the WITH_GNUTLS macro.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/rpc/virnetserverclient.h | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John
participants (2)
-
Daniel P. Berrangé -
John Ferlan