[libvirt] [PATCH 3/7] remote generator: Make call-by-reference handling stricter
Daniel P. Berrange
berrange at redhat.com
Wed May 25 16:11:14 UTC 2011
On Mon, May 23, 2011 at 07:36:06PM +0200, Matthias Bolte wrote:
> Several functions return values by reference parameters. This is realized
> by passing the members of remote_CALL_ret by reference to the called
> function.
>
> The position of this parameters in the function signature follows some
> patterns with some exceptions. This patterns and exceptions are hardcoded
> in the generator.
>
> Add an insert@<offset> annotation to the remote_CALL_ret struct members
> for functions that return lists to remove some of the hardcoded patterns
> and exceptions.
> ---
> daemon/remote_generator.pl | 69 ++++++++++++++---------------------------
> src/remote/remote_protocol.x | 37 ++++++++++++----------
> 2 files changed, 44 insertions(+), 62 deletions(-)
>
> diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl
> index 2da0f2e..8b3794f 100755
> --- a/daemon/remote_generator.pl
> +++ b/daemon/remote_generator.pl
> @@ -405,8 +405,9 @@ elsif ($opt_b) {
> } else {
> die "unhandled type for multi-return-value: $ret_member";
> }
> - } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<(\S+)>;/) {
> + } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
> push(@vars_list, "int len");
> + splice(@args_list, int($3), 0, ("ret->$1.$1_val"));
> push(@ret_list, "ret->$1.$1_len = len;");
> push(@free_list_on_error, "VIR_FREE(ret->$1.$1_val);");
> $single_ret_var = "len";
> @@ -416,18 +417,9 @@ elsif ($opt_b) {
> $single_ret_list_name = $1;
> $single_ret_list_max_var = "max$1";
> $single_ret_list_max_define = $2;
> -
> - if ($call->{ProcName} eq "NodeListDevices") {
> - my $conn = shift(@args_list);
> - my $cap = shift(@args_list);
> - unshift(@args_list, "ret->$1.$1_val");
> - unshift(@args_list, $cap);
> - unshift(@args_list, $conn);
> - } else {
> - my $conn = shift(@args_list);
> - unshift(@args_list, "ret->$1.$1_val");
> - unshift(@args_list, $conn);
> - }
> + } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<\S+>;/) {
> + # error out on unannotated arrays
> + die "remote_nonnull_string array without insert@<offset> annotation: $ret_member";
> } elsif ($ret_member =~ m/^remote_nonnull_string (\S+);/) {
> if ($call->{ProcName} eq "GetType") {
> # SPECIAL: virConnectGetType returns a constant string that must
> @@ -466,8 +458,9 @@ elsif ($opt_b) {
> $single_ret_by_ref = 0;
> $single_ret_check = " == NULL";
> }
> - } elsif ($ret_member =~ m/^int (\S+)<(\S+)>;/) {
> + } elsif ($ret_member =~ m/^int (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
> push(@vars_list, "int len");
> + splice(@args_list, int($3), 0, ("ret->$1.$1_val"));
> push(@ret_list, "ret->$1.$1_len = len;");
> push(@free_list_on_error, "VIR_FREE(ret->$1.$1_val);");
> $single_ret_var = "len";
> @@ -477,10 +470,9 @@ elsif ($opt_b) {
> $single_ret_list_name = $1;
> $single_ret_list_max_var = "max$1";
> $single_ret_list_max_define = $2;
> -
> - my $conn = shift(@args_list);
> - unshift(@args_list, "ret->$1.$1_val");
> - unshift(@args_list, $conn);
> + } elsif ($ret_member =~ m/^int (\S+)<\S+>;/) {
> + # error out on unannotated arrays
> + die "int array without insert@<offset> annotation: $ret_member";
> } elsif ($ret_member =~ m/^int (\S+);/) {
> push(@vars_list, "int $1");
> push(@ret_list, "ret->$1 = $1;");
> @@ -497,7 +489,7 @@ elsif ($opt_b) {
> $single_ret_check = " < 0";
> }
> }
> - } elsif ($ret_member =~ m/^hyper (\S+)<(\S+)>;/) {
> + } elsif ($ret_member =~ m/^hyper (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
> push(@vars_list, "int len");
> push(@ret_list, "ret->$1.$1_len = len;");
> push(@free_list_on_error, "VIR_FREE(ret->$1.$1_val);");
> @@ -508,17 +500,16 @@ elsif ($opt_b) {
> $single_ret_list_max_var = "max$1";
> $single_ret_list_max_define = $2;
>
> - my $conn = shift(@args_list);
> -
> if ($call->{ProcName} eq "NodeGetCellsFreeMemory") {
> $single_ret_check = " <= 0";
> - unshift(@args_list, "(unsigned long long *)ret->$1.$1_val");
> + splice(@args_list, int($3), 0, ("(unsigned long long *)ret->$1.$1_val"));
> } else {
> $single_ret_check = " < 0";
> - unshift(@args_list, "ret->$1.$1_val");
> + splice(@args_list, int($3), 0, ("ret->$1.$1_val"));
> }
> -
> - unshift(@args_list, $conn);
> + } elsif ($ret_member =~ m/^hyper (\S+)<\S+>;/) {
> + # error out on unannotated arrays
> + die "hyper array without insert@<offset> annotation: $ret_member";
> } elsif ($ret_member =~ m/^(unsigned )?hyper (\S+);/) {
> my $type_name;
> my $ret_name = $2;
> @@ -945,30 +936,18 @@ elsif ($opt_k) {
> die "unhandled type for multi-return-value for " .
> "procedure $call->{name}: $ret_member";
> }
> - } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<(\S+)>;/) {
> + } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<(\S+)>;\s*\/\*\s*insert@(\d+)\s*\*\//) {
> + splice(@args_list, int($3), 0, ("char **const $1"));
> + push(@ret_list, "rv = ret.$1.$1_len;");
> + $single_ret_var = "int rv = -1";
> + $single_ret_type = "int";
> $single_ret_as_list = 1;
> $single_ret_list_name = $1;
> $single_ret_list_max_var = "max$1";
> $single_ret_list_max_define = $2;
> -
> - my $first_arg = shift(@args_list);
> - my $second_arg;
> -
> - if ($call->{ProcName} eq "NodeListDevices") {
> - $second_arg = shift(@args_list);
> - }
> -
> - unshift(@args_list, "char **const $1");
> -
> - if (defined $second_arg) {
> - unshift(@args_list, $second_arg);
> - }
> -
> - unshift(@args_list, $first_arg);
> -
> - push(@ret_list, "rv = ret.$1.$1_len;");
> - $single_ret_var = "int rv = -1";
> - $single_ret_type = "int";
> + } elsif ($ret_member =~ m/^remote_nonnull_string (\S+)<\S+>;/) {
> + # error out on unannotated arrays
> + die "remote_nonnull_string array without insert@<offset> annotation: $ret_member";
> } elsif ($ret_member =~ m/^remote_nonnull_string (\S+);/) {
> push(@ret_list, "rv = ret.$1;");
> $single_ret_var = "char *rv = NULL";
> diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
> index 3e9bd5c..63f7ebb 100644
> --- a/src/remote/remote_protocol.x
> +++ b/src/remote/remote_protocol.x
> @@ -368,7 +368,10 @@ struct remote_memory_param {
> *
> * Please follow the naming convention carefully - this file is
> * parsed by 'remote_generator.pl'.
> - */
> + *
> + * 'remote_CALL_ret' members that are filled via call-by-reference must be
> + * annotated with a insert@<offset> comment to indicate the offset in the
> + * parameter list of the function to be called. */
>
> struct remote_open_args {
> /* NB. "name" might be NULL although in practice you can't
> @@ -446,7 +449,7 @@ struct remote_node_get_cells_free_memory_args {
> };
>
> struct remote_node_get_cells_free_memory_ret {
> - hyper cells<REMOTE_NODE_MAX_CELLS>;
> + hyper cells<REMOTE_NODE_MAX_CELLS>; /* insert at 1 */
> };
>
> struct remote_node_get_free_memory_ret {
> @@ -600,7 +603,7 @@ struct remote_list_domains_args {
> };
>
> struct remote_list_domains_ret {
> - int ids<REMOTE_DOMAIN_ID_LIST_MAX>;
> + int ids<REMOTE_DOMAIN_ID_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_num_of_domains_ret {
> @@ -801,7 +804,7 @@ struct remote_list_defined_domains_args {
> };
>
> struct remote_list_defined_domains_ret {
> - remote_nonnull_string names<REMOTE_DOMAIN_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_DOMAIN_NAME_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_num_of_defined_domains_ret {
> @@ -949,7 +952,7 @@ struct remote_list_networks_args {
> };
>
> struct remote_list_networks_ret {
> - remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_num_of_defined_networks_ret {
> @@ -961,7 +964,7 @@ struct remote_list_defined_networks_args {
> };
>
> struct remote_list_defined_networks_ret {
> - remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_NETWORK_NAME_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_network_lookup_by_uuid_args {
> @@ -1049,7 +1052,7 @@ struct remote_list_nwfilters_args {
> };
>
> struct remote_list_nwfilters_ret {
> - remote_nonnull_string names<REMOTE_NWFILTER_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_NWFILTER_NAME_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_nwfilter_lookup_by_uuid_args {
> @@ -1101,7 +1104,7 @@ struct remote_list_interfaces_args {
> };
>
> struct remote_list_interfaces_ret {
> - remote_nonnull_string names<REMOTE_INTERFACE_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_INTERFACE_NAME_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_num_of_defined_interfaces_ret {
> @@ -1113,7 +1116,7 @@ struct remote_list_defined_interfaces_args {
> };
>
> struct remote_list_defined_interfaces_ret {
> - remote_nonnull_string names<REMOTE_DEFINED_INTERFACE_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_DEFINED_INTERFACE_NAME_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_interface_lookup_by_name_args {
> @@ -1215,7 +1218,7 @@ struct remote_list_storage_pools_args {
> };
>
> struct remote_list_storage_pools_ret {
> - remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_num_of_defined_storage_pools_ret {
> @@ -1227,7 +1230,7 @@ struct remote_list_defined_storage_pools_args {
> };
>
> struct remote_list_defined_storage_pools_ret {
> - remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_STORAGE_POOL_NAME_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_find_storage_pool_sources_args {
> @@ -1357,7 +1360,7 @@ struct remote_storage_pool_list_volumes_args {
> };
>
> struct remote_storage_pool_list_volumes_ret {
> - remote_nonnull_string names<REMOTE_STORAGE_VOL_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_STORAGE_VOL_NAME_LIST_MAX>; /* insert at 1 */
> };
>
>
> @@ -1465,7 +1468,7 @@ struct remote_node_list_devices_args {
> };
>
> struct remote_node_list_devices_ret {
> - remote_nonnull_string names<REMOTE_NODE_DEVICE_NAME_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_NODE_DEVICE_NAME_LIST_MAX>; /* insert at 2 */
> };
>
> struct remote_node_device_lookup_by_name_args {
> @@ -1507,7 +1510,7 @@ struct remote_node_device_list_caps_args {
> };
>
> struct remote_node_device_list_caps_ret {
> - remote_nonnull_string names<REMOTE_NODE_DEVICE_CAPS_LIST_MAX>;
> + remote_nonnull_string names<REMOTE_NODE_DEVICE_CAPS_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_node_device_dettach_args {
> @@ -1588,7 +1591,7 @@ struct remote_list_secrets_args {
> };
>
> struct remote_list_secrets_ret {
> - remote_nonnull_string uuids<REMOTE_SECRET_UUID_LIST_MAX>;
> + remote_nonnull_string uuids<REMOTE_SECRET_UUID_LIST_MAX>; /* insert at 1 */
> };
>
> struct remote_secret_lookup_by_uuid_args {
> @@ -1900,7 +1903,7 @@ struct remote_domain_snapshot_list_names_args {
> };
>
> struct remote_domain_snapshot_list_names_ret {
> - remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>;
> + remote_nonnull_string names<REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX>; /* insert at 1 */
> };
>
> struct remote_domain_snapshot_lookup_by_name_args {
> @@ -2006,7 +2009,7 @@ struct remote_domain_migrate_prepare_tunnel3_args {
> };
>
> struct remote_domain_migrate_prepare_tunnel3_ret {
> - opaque cookie_out<REMOTE_MIGRATE_COOKIE_MAX>;
> + opaque cookie_out<REMOTE_MIGRATE_COOKIE_MAX>; /* insert at 3 */
> };
>
> struct remote_domain_migrate_perform3_args {
ACK
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 :|
More information about the libvir-list
mailing list