[libvirt] [PATCH] sendkey: use consistent API convention

[Daniel P. Berrange]

On Wed, Jun 15, 2011 at 10:37:49AM -0600, Eric Blake wrote:
> Even though rpc uses 'unsigned int' for the _val parameter that
> passes the length of item<length>, the public libvirt APIs all
> use 'int' and filter out lengths < 0, except for virDomainSendKey.
> 
> * include/libvirt/libvirt.h.in (virDomainSendKey): All other APIs
> use int for array length.
> * src/libvirt.c (virDomainSendKey): Adjust.
> * src/driver.h (virDrvDomainSendKey): Likewise.
> * daemon/remote_generator.pl: Likewise.
> ---
> 
> One approach to a question first raised here.
> https://www.redhat.com/archives/libvir-list/2011-June/msg00681.html
> 
> The other approach would be to change all libvirt API to use
> unsigned int for array sizes, to match rpc conventions and to
> simplify code to not have to worry about negative sizes, but
> that is more invasive.
> 
>  daemon/remote_generator.pl   |    2 +-
>  include/libvirt/libvirt.h.in |    2 +-
>  src/driver.h                 |    2 +-
>  src/libvirt.c                |    4 ++--
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
> index 351866b..afec66c 100755
> --- a/daemon/remote_generator.pl
> +++ b/daemon/remote_generator.pl
> @@ -1007,7 +1007,7 @@ elsif ($opt_k) {
>                      my $limit = $3;
> 
>                      push(@args_list, "${type_name} *$arg_name");
> -                    push(@args_list, "unsigned int ${arg_name}len");
> +                    push(@args_list, "int ${arg_name}len");
>                      push(@setters_list, "args.$arg_name.${arg_name}_val = $arg_name;");
>                      push(@setters_list, "args.$arg_name.${arg_name}_len = ${arg_name}len;");
>                      push(@args_check_list, { name => "\"$arg_name\"", arg => "${arg_name}len", limit => $limit });
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index ab22046..c684297 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1788,7 +1788,7 @@ int virDomainSendKey(virDomainPtr domain,
>                       unsigned int codeset,
>                       unsigned int holdtime,
>                       unsigned int *keycodes,
> -                     unsigned int nkeycodes,
> +                     int nkeycodes,
>                       unsigned int flags);
> 
>  /*
> diff --git a/src/driver.h b/src/driver.h
> index d45575a..c880222 100644
> --- a/src/driver.h
> +++ b/src/driver.h
> @@ -568,7 +568,7 @@ typedef int
>      (*virDrvDomainSendKey)(virDomainPtr dom, unsigned int codeset,
>                             unsigned int holdtime,
>                             unsigned int *keycodes,
> -                           unsigned int nkeycodes,
> +                           int nkeycodes,
>                             unsigned int flags);
> 
>  typedef char *
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 36a90d1..b2e1d02 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -6653,7 +6653,7 @@ int virDomainSendKey(virDomainPtr domain,
>                       unsigned int codeset,
>                       unsigned int holdtime,
>                       unsigned int *keycodes,
> -                     unsigned int nkeycodes,
> +                     int nkeycodes,
>                       unsigned int flags)
>  {
>      virConnectPtr conn;
> @@ -6663,7 +6663,7 @@ int virDomainSendKey(virDomainPtr domain,
>      virResetLastError();
> 
>      if (keycodes == NULL ||
> -        nkeycodes == 0 || nkeycodes > VIR_DOMAIN_SEND_KEY_MAX_KEYS) {
> +        nkeycodes <= 0 || nkeycodes > VIR_DOMAIN_SEND_KEY_MAX_KEYS) {
>          virLibDomainError(VIR_ERR_OPERATION_INVALID, __FUNCTION__);
>          virDispatchError(NULL);
>          return -1;

ACK,

If we were starting the API from scratch, I'd say we
should have used 'unsigned int' everywhere. Given our
existing practice though, we should stick with 'int'
everywhere.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|