Re: [libvirt] [PATCH 3/7] remote generator: Make call-by-reference handling stricter

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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@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@3 */ }; struct remote_domain_migrate_perform3_args {