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 :|