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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list