Refactor _parse_varvals.

Now it takes two arguments: A regexp and a string to match against.
_parse_varvals itself was split in three:  _preprocess_varvals,
_parse_varvals, and _process_val.

This also modifies the three places that call _parse_varvals; For
two, no real changes were needed, but parse_mysqld() required a fix
to deal with the two final lines of mysqld --help --verbose:

   To see what values a running MySQL server is using, type
   'mysqladmin variables' instead of 'mysqld --verbose --help'.

bin/pt-config-diff

@@ -2145,15 +2145,16 @@ sub parse_mysqld {
       PTDEBUG && _d("mysqld help output doesn't list option files");
    }
 
-   if ( $output !~ m/^-+ -+$/mg ) {
+   if ( $output !~ m/^-+ -+$(.+?)(?:\n\n.+)?\z/sm ) {
       PTDEBUG && _d("mysqld help output doesn't list vars and vals");
       return;
    }
 
-   my $varvals = substr($output, (pos $output) + 1, length $output);
+   my $varvals = $1;
 
    my ($config, undef) = _parse_varvals(
-      $varvals =~ m/\G^(\S+)(.*)\n/mg
+      qr/^(\S+)(.*)$/,
+      $varvals,
    );
 
    return $config, \@opt_files;
@@ -2168,9 +2169,8 @@ sub parse_my_print_defaults {
    my ($output) = @args{@required_args};
 
    my ($config, $dupes) = _parse_varvals(
-      map  { $_ =~ m/^--([^=]+)(?:=(.*))?$/ ? ($1, $2) : () }
+      qr/^--([^=]+)(?:=(.*))?$/,
-      grep { $_ !~ m/^\s*$/ }  # no empty lines
+      $output,
-      split("\n", $output)
    );
 
    return $config, $dupes;
@@ -2188,53 +2188,72 @@ sub parse_option_file {
    die "Failed to parse the [mysqld] section" unless $mysqld_section;
 
    my ($config, $dupes) = _parse_varvals(
-      map  { $_ =~ m/^([^=]+)(?:=(.*))?$/ ? ($1, $2) : () }
+      qr/^([^=]+)(?:=(.*))?$/,
-      grep { $_ !~ m/^\s*$/ }  # no empty lines
+      $mysqld_section,
-      grep { $_ !~ m/^\s*#/ }  # no # comment lines
-      split("\n", $mysqld_section)
    );
 
    return $config, $dupes;
 }
 
-sub _parse_varvals {
+sub _preprocess_varvals {
-   my ( @varvals ) = @_;
+   my ($re, $to_parse) = @_;
 
-   my %config;
+   my %vars;
+   LINE:
+   foreach my $line ( split /\n/, $to_parse ) {
+      next LINE if $line =~ m/^\s*$/;  # no empty lines
+      next LINE if $line =~ m/^\s*#/;  # no # comment lines
 
-   my $duplicate_var = 0;
+      if ( $line !~ $re ) {
-   my %duplicates;
+         PTDEBUG && _d("Line <", $line, "> didn't match $re");
-
+         next LINE;
-   my $var;  # current variable (e.g. datadir)
-   my $val;  # value for current variable
-   ITEM:
-   foreach my $item ( @varvals ) {
-      if ( !defined($item) ) {
-         $item = '';
-      }
-      if ( $item ) {
-         $item =~ s/^\s+//;
-         $item =~ s/\s+$//;
       }
 
-      if ( !$var ) {
+      my ($var, $val) = ($1, $2);
-         $var = $item;
       
-         $var =~ s/-/_/g;
+      $var =~ tr/-/_/;
-
-         if ( exists $config{$var} && !$can_be_duplicate{$var} ) {
-            PTDEBUG && _d("Duplicate var:", $var);
-            $duplicate_var = 1;  # flag on, save all the var's values
-         }
-      }
-      else {
-         my $val = $item;
-         PTDEBUG && _d("Var:", $var, "val:", $val);
 
       if ( !defined $val ) {
          $val = '';
       }
-         else {
+      
+      for my $item ($var, $val) {
+         $item =~ s/^\s+//;
+         $item =~ s/\s+$//;
+      }
+
+      push @{$vars{$var} ||= []}, $val
+   }
+
+   return \%vars;
+}
+
+sub _parse_varvals {
+   my ( $vars ) = _preprocess_varvals(@_);
+
+   my %config;
+
+   my %duplicates;
+
+   while ( my ($var, $vals) = each %$vars ) {
+      my $val = _process_val( pop @$vals );
+      if ( @$vals && !$can_be_duplicate{$var} ) {
+         PTDEBUG && _d("Duplicate var:", $var);
+         foreach my $current_val ( map { _process_val($_) } @$vals ) {
+            push @{$duplicates{$var} ||= []}, $current_val;
+         }
+      }
+
+      PTDEBUG && _d("Var:", $var, "val:", $val);
+
+      $config{$var} = $val;
+   }
+
+   return \%config, \%duplicates;
+}
+
+sub _process_val {
+   my ($val) = @_;
    $val =~ s/
                \A        # Start of value
                (['"])    # Opening quote
@@ -2254,20 +2273,7 @@ sub _parse_varvals {
    elsif ( $val =~ m/No default/ ) {
       $val = '';
    }
-         }
+   return $val;
-
-         if ( $duplicate_var ) {
-            push @{$duplicates{$var}}, $config{$var};
-            $duplicate_var = 0;  # flag off for next var
-         }
-
-         $config{$var} = $val;
-
-         $var = undef;  # next item should be a var
-      }
-   }
-
-   return \%config, \%duplicates;
 }
 
 sub _mimic_show_variables {

lib/MySQLConfig.pm

@@ -259,18 +259,28 @@ sub parse_mysqld {
    #   help                              TRUE
    #   abort-slave-event-count           0
    # So we search for that line of hypens.
-   if ( $output !~ m/^-+ -+$/mg ) {
+   #
+   # It also ends with something like
+   #
+   #   wait_timeout                      28800
+   #   
+   #   To see what values a running MySQL server is using, type
+   #   'mysqladmin variables' instead of 'mysqld --verbose --help'.
+   #
+   # So try to find it by locating a double newline, and strip it away
+   if ( $output !~ m/^-+ -+$(.+?)(?:\n\n.+)?\z/sm ) {
       PTDEBUG && _d("mysqld help output doesn't list vars and vals");
       return;
    }
 
-   # Cut off everything before the list of vars and vals.
+   # Grab the varval list.
-   my $varvals = substr($output, (pos $output) + 1, length $output);
+   my $varvals = $1;
 
    # Parse the "var  val" lines.  2nd retval is duplicates but there
    # shouldn't be any with mysqld.
    my ($config, undef) = _parse_varvals(
-      $varvals =~ m/\G^(\S+)(.*)\n/mg
+      qr/^(\S+)(.*)$/,
+      $varvals,
    );
 
    return $config, \@opt_files;
@@ -288,9 +298,8 @@ sub parse_my_print_defaults {
 
    # Parse the "--var=val" lines.
    my ($config, $dupes) = _parse_varvals(
-      map  { $_ =~ m/^--([^=]+)(?:=(.*))?$/ ? ($1, $2) : () }
+      qr/^--([^=]+)(?:=(.*))?$/,
-      grep { $_ !~ m/^\s*$/ }  # no empty lines
+      $output,
-      split("\n", $output)
    );
 
    return $config, $dupes;
@@ -311,74 +320,93 @@ sub parse_option_file {
 
    # Parse the "var=val" lines.
    my ($config, $dupes) = _parse_varvals(
-      map  { $_ =~ m/^([^=]+)(?:=(.*))?$/ ? ($1, $2) : () }
+      qr/^([^=]+)(?:=(.*))?$/,
-      grep { $_ !~ m/^\s*$/ }  # no empty lines
+      $mysqld_section,
-      grep { $_ !~ m/^\s*#/ }  # no # comment lines
-      split("\n", $mysqld_section)
    );
 
    return $config, $dupes;
 }
 
-# Parses a list of variables and their values ("varvals"), returns two
+# Called by _parse_varvals(), takes two arguments: a regex, and
+# a string to match against. The string will be split in lines,
+# and each line will be matched against the regex.
+# The regex must return to captures, although the second doesn't
+# have to match anything.
+# Returns a hashref of arrayrefs ala
+# { port => [ 12345, 12346 ], key_buffer_size => [ "16M" ] }
+sub _preprocess_varvals {
+   my ($re, $to_parse) = @_;
+
+   my %vars;
+   LINE:
+   foreach my $line ( split /\n/, $to_parse ) {
+      next LINE if $line =~ m/^\s*$/;  # no empty lines
+      next LINE if $line =~ m/^\s*#/;  # no # comment lines
+
+      if ( $line !~ $re ) {
+         PTDEBUG && _d("Line <", $line, "> didn't match $re");
+         next LINE;
+      }
+
+      my ($var, $val) = ($1, $2);
+      
+      # Variable names are usually specified like "log-bin"
+      # but in SHOW VARIABLES they're all like "log_bin".
+      $var =~ tr/-/_/;
+
+      if ( !defined $val ) {
+         $val = '';
+      }
+      
+      # Strip leading and trailing whitespace.
+      for my $item ($var, $val) {
+         $item =~ s/^\s+//;
+         $item =~ s/\s+$//;
+      }
+
+      push @{$vars{$var} ||= []}, $val
+   }
+
+   return \%vars;
+}
+
+# Parses a string of variables and their values ("varvals"), returns two
 # hashrefs: one with normalized variable=>value, the other with duplicate
-# vars.  The varvals list should start with a var at index 0 and its value
+# vars.
-# at index 1 then repeat for the next var-val pair.  
 sub _parse_varvals {
-   my ( @varvals ) = @_;
+   my ( $vars ) = _preprocess_varvals(@_);
 
    # Config built from parsing the given varvals.
    my %config;
 
    # Discover duplicate vars.  
-   my $duplicate_var = 0;
    my %duplicates;
 
-   # Keep track if item is var or val because each needs special modifications.
+   while ( my ($var, $vals) = each %$vars ) {
-   my $var;  # current variable (e.g. datadir)
+      my $val = _process_val( pop @$vals );
-   my $val;  # value for current variable
+      # If the variable has duplicates, then @$vals will contain
-   ITEM:
+      # the rest of the values
-   foreach my $item ( @varvals ) {
+      if ( @$vals && !$can_be_duplicate{$var} ) {
-      # We were passed an undef for the value, or we're dealing with an empty
-      # line
-      if ( !defined($item) ) {
-         $item = '';
-      }
-      if ( $item ) {
-         # Strip leading and trailing whitespace.
-         $item =~ s/^\s+//;
-         $item =~ s/\s+$//;
-      }
-
-      if ( !$var ) {
-         # No var means this item is (should be) the next var in the list.
-         $var = $item;
-
-         # Variable names are usually specified like "log-bin"
-         # but in SHOW VARIABLES they're all like "log_bin".
-         $var =~ s/-/_/g;
-
          # The var is a duplicate (in the bad sense, i.e. where user is
          # probably unaware that there's two different values for this var
-         # but only the last is used) if we've seen it already and it cannot
+         # but only the last is used).
-         # be duplicated.  We don't have its value yet (next loop iter),
-         # so we set a flag to indicate that we should save the duplicate value.
-         if ( exists $config{$var} && !$can_be_duplicate{$var} ) {
          PTDEBUG && _d("Duplicate var:", $var);
-            $duplicate_var = 1;  # flag on, save all the var's values
+         foreach my $current_val ( map { _process_val($_) } @$vals ) {
+            push @{$duplicates{$var} ||= []}, $current_val;
          }
       }
-      else {
+
-         # $var is set so this item should be its value.
-         my $val = $item;
       PTDEBUG && _d("Var:", $var, "val:", $val);
 
-         # Avoid crashing on undef comparison.  Also, SHOW VARIABLES uses
+      # Save this var-val.
-         # blank strings, not NULL/undef.
+      $config{$var} = $val;
-         if ( !defined $val ) {
-            $val = '';
    }
-         else {
+
+   return \%config, \%duplicates;
+}
+
+sub _process_val {
+   my ($val) = @_;
    $val =~ s/
                \A        # Start of value
                (['"])    # Opening quote
@@ -399,23 +427,7 @@ sub _parse_varvals {
    elsif ( $val =~ m/No default/ ) {
       $val = '';
    }
-         }
+   return $val;
-
-         if ( $duplicate_var ) {
-            # Save the var's last value before we overwrite it with this
-            # current value.
-            push @{$duplicates{$var}}, $config{$var};
-            $duplicate_var = 0;  # flag off for next var
-         }
-
-         # Save this var-val.
-         $config{$var} = $val;
-
-         $var = undef;  # next item should be a var
-      }
-   }
-
-   return \%config, \%duplicates;
 }
 
 # Sub: _mimic_show_variables