[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]