Re: [PATCH 3/5] util: Export remoteDeserializeTypedParameters internally via util

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Feb 02, 2016 at 04:46:19PM +0100, Erik Skultety wrote:
> Currently, the deserializer is hardcoded into remote_driver which makes
> it impossible for admin to use it. One way to achieve a shared implementation
> (besides moving the code to another module) would be pass @ret_params_val as a
> void pointer as opposed to the remote_typed_param pointer and add a new extra
> argument specifying which of those two protocols is being used and typecast
> the pointer at the function entry. An example from remote_protocol:
> 
> struct remote_typed_param_value {
>         int type;
>         union {
>                 int i;
>                 u_int ui;
>                 int64_t l;
>                 uint64_t ul;
>                 double d;
>                 int b;
>                 remote_nonnull_string s;
>         } remote_typed_param_value_u;
> };
> typedef struct remote_typed_param_value remote_typed_param_value;
> 
> struct remote_typed_param {
>         remote_nonnull_string field;
>         remote_typed_param_value value;
> };
> 
> That would leave us with a bunch of if-then-elses that needed to be used across
> the method. This patch takes the other approach using the new datatype
> introduced in one of earlier commits.
> ---
>  daemon/remote.c            | 125 +++++++++-----------------------------------
>  src/libvirt_private.syms   |   1 +
>  src/remote/remote_driver.c | 107 ++------------------------------------
>  src/rpc/gendispatch.pl     |   9 ++--
>  src/util/virtypedparam.c   | 127 +++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virtypedparam.h   |   7 +++
>  6 files changed, 169 insertions(+), 207 deletions(-)

> @@ -1747,105 +1749,6 @@ remoteSerializeTypedParameters(virTypedParameterPtr params,
>      return rv;
>  }
>  
> -/* Helper to deserialize typed parameters. */
> -static int
> -deserializeTypedParameters(const char *funcname,

What is the point of the funcname parameter?
Dropping it first would make it easier to unify the code paths.

> +    /* Deserialise the result. */
> +    for (i = 0; i < remote_params_len; ++i) {
> +        virTypedParameterPtr param = *params + i;
> +        virTypedParameterRemotePtr remote_param = remote_params + i;

Please use (*params)[i] and remote_params[i].

Jan

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]