Re: [libvirt] [PATCH 1/6] Cleanup command line options in gendispatch.pl

Tuesday, 23 April 2013

On 04/23/2013 04:26 AM, Daniel P. Berrange wrote:
...
 From: "Daniel P. Berrange" <berrange(a)redhat.com&gt;

 There are many declared options in gendispatch.pl that were
 no longer used. Those which were used were obsecure '-b', '-k' 
s/obsecure/obscure/

...
 and '-d'. Switch to use --mode={debug|client|server}.

 Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt;
 ---
  daemon/Makefile.am     | 12 +++----
  src/Makefile.am        | 24 ++++++-------
  src/rpc/gendispatch.pl | 94 ++++++++++++++++++++++++--------------------------
  3 files changed, 63 insertions(+), 67 deletions(-) 
...
 +++ b/src/rpc/gendispatch.pl 
...
 +        # for now, we distinguish only two levels of prioroty:

s/prioroty/priority/ while reindenting.

Beyond that, I found it much easier to read with 'git diff -b':

> diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
> index 65a0ac2..fdc345f 100755
> --- a/src/rpc/gendispatch.pl
...
 +++ b/src/rpc/gendispatch.pl > @@ -17,13 +17,15 @@
> 
>  use strict;
> 
> -use Getopt::Std;
> +use Getopt::Long;
> 
> -# Command line options.
> -# -k - client bodies
> -# -b - server bodies
> -our ($opt_p, $opt_t, $opt_a, $opt_r, $opt_d, $opt_b, $opt_k);
> -getopts ('ptardbk');
> +my $mode = "debug";
> +my $res = GetOptions("mode=s" => \$mode);
> +
> +die "cannot parse command line options" unless $res;
> +
> +die "unknown mode '$mode', expecting 'client', 'server'
or 'debug'"
> +    unless $mode =~ /^(client|server|debug)$/;
> 
>  my $structprefix = shift or die "missing struct prefix argument";
>  my $procprefix = shift or die "missing procedure prefix argument";
> @@ -105,7 +107,7 @@ while (<PROTOCOL>) {
>              push(@{$calls{$name}->{ret_members}}, $1);
>          }
>      } elsif ($collect_opts) {
> -        if (m,^\s*\*\s*\@(\w+)\s*:\s*(\w+)\s*$,) {
> +        if (m,^\s*\*\s*\@(\w+)\s*:\s*((?:\w|:)+)\s*$,) {

Does this change really belong here, or is it a separate cleanup better
put in its own patch?

...
              $opts{$1} = $2;
          } elsif (m,^\s*\*/\s*$,) {
              $collect_opts = 0;
 @@ -159,7 +161,7 @@ while (<PROTOCOL>) {
              }
          }

 -        if ($flags ne "" and ($opt_b or $opt_k)) {
 +        if ($flags ne "") {
              if (!($flags =~ m/^\s*\/\*\s*insert@(\d+)\s*\*\/\s*$/)) {
                  die "invalid generator flags for $calls{$name}->{ret}";
              }
 @@ -207,7 +209,6 @@ while (<PROTOCOL>) {
          }
          $calls{$name}->{constname} = $constname;

 -        if ($opt_b or $opt_k) {
          if (!exists $opts{generate}) {
              die "'\@generate' annotation missing for $constname";
          }
 @@ -218,9 +219,9 @@ while (<PROTOCOL>) {

          if ($opts{generate} eq "both") {
              push(@autogen, $ProcName);
 -            } elsif ($opt_b && ($opts{generate} eq "server")) {
 +        } elsif ($mode eq "server" && ($opts{generate} eq
"server")) {
              push(@autogen, $ProcName);
 -            } elsif (!$opt_b && ($opts{generate} eq "client")) {
 +        } elsif ($mode eq "client" && ($opts{generate} eq
"client")) {
              push(@autogen, $ProcName);
          }

 @@ -248,7 +249,6 @@ while (<PROTOCOL>) {
          } else {
              $calls{$name}->{priority} = 0;
          }
 -        }

          $calls[$id] = $calls{$name};

 @@ -327,12 +327,8 @@ print <<__EOF__;
   */
  __EOF__

 -if (!$opt_b and !$opt_k) {
 -    print "\n";
 -}
 -
  # Debugging.
 -if ($opt_d) {
 +if ($mode eq "debug") {
      my @keys = sort (keys %calls);
      foreach (@keys) {
          print "$_:\n";
 @@ -343,7 +339,7 @@ if ($opt_d) {
  }

  # Bodies for dispatch functions ("remote_dispatch_bodies.h").
 -elsif ($opt_b) {
 +elsif ($mode eq "server") {
      my %generate = map { $_ => 1 } @autogen;
      my @keys = sort (keys %calls);

 @@ -1050,7 +1046,7 @@ elsif ($opt_b) {
  }

  # Bodies for client functions ("remote_client_bodies.h").
 -elsif ($opt_k) {
 +elsif ($mode eq "client") {
      my %generate = map { $_ => 1 } @autogen;
      my @keys = sort (keys %calls);

ACK with typos fixed.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

2005

Re: [libvirt] [PATCH 1/6] Cleanup command line options in gendispatch.pl