[libvirt] [PATCH 1/3] tests: validate private data / pre / post exec hooks for RPC APIs

[John Ferlan]

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 at 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 at 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;
>      }
>