Re: perl module documentation patches, segfault

[John Darrington]

On Sun, Feb 01, 2009 at 07:10:42PM -0800, Ben Pfaff wrote:

     I figured it out.  The get_next_case() implementation was
     returning a null pointer when cases were exhausted.  Apparently
     you are not supposed to do that.
     
     How about the following commit to both fix this and return an
     array instead of a reference?


Okay.  Can you also change the version number to 0.7.2, since I know
there's at least one party whose using the old interface.
     
     commit 22ee9b2b06bdc68170128a3e7085e84c34a26c84
     Author: Ben Pfaff <address@hidden>
     Date:   Sun Feb 1 19:00:21 2009 -0800
     
         perl-module: Make PSPP::Reader::get_next_case() return a list.
         
         PSPP::Reader::get_next_case() was documented to return a list,
         but actually it returned a reference to a list.  This commit
         changes the behavior to match the documentation.
         
         This also fixes the behavior on reading past the end of the file.
         Previously this was documented to return undef but caused a
         segfault in practice.  Now it returns an empty list.
     
     diff --git a/perl-module/PSPP.xs b/perl-module/PSPP.xs
     index 2d2c8d9..237ae95 100644
     --- a/perl-module/PSPP.xs
     +++ b/perl-module/PSPP.xs
     @@ -671,32 +671,24 @@ CODE:
      RETVAL
      
      
     -SV *
     +void
      get_next_case (sfr)
       struct sysreader_info *sfr;
     -CODE:
     +PPCODE:
       struct ccase *c;
      
     - if (! (c = casereader_read (sfr->reader)))
     - {
     -  RETVAL = 0;
     - }
     - else
     + if (c = casereader_read (sfr->reader))
       {
        int v;
     -  AV *av_case = (AV *) sv_2mortal ((SV *) newAV());
      
     +  EXTEND (SP, dict_get_var_cnt (sfr->dict));
        for (v = 0; v < dict_get_var_cnt (sfr->dict); ++v )
          {
            const struct variable *var = dict_get_var (sfr->dict, v);
            const union value *val = case_data (c, var);
      
     -      av_push (av_case, value_to_scalar (val, var));
     +      PUSHs (sv_2mortal (value_to_scalar (val, var)));
          }
      
        case_unref (c);
     -  RETVAL = newRV ((SV *) av_case);
       }
     -OUTPUT:
     - RETVAL
     -
     diff --git a/perl-module/lib/PSPP.pm b/perl-module/lib/PSPP.pm
     index 93d70ef..e9e3215 100644
     --- a/perl-module/lib/PSPP.pm
     +++ b/perl-module/lib/PSPP.pm
     @@ -493,7 +493,8 @@ This method returns an array of scalars, each of which 
are the values of
      the data in the system file.
      The first call to C<get_next_case> after C<open> has been called retrieves
      the first case in the system file.  Each subsequent call retrieves the 
next
     -case.  If there are no more cases to be read, the function returns undef.
     +case.  If there are no more cases to be read, the function returns an 
empty
     +list.
      
      If the case contains system missing values, these values are set to the 
      empty string.
     diff --git a/perl-module/t/Pspp.t b/perl-module/t/Pspp.t
     index fe14151..030a342 100644
     --- a/perl-module/t/Pspp.t
     +++ b/perl-module/t/Pspp.t
     @@ -387,11 +387,11 @@ RESULT
          print MYFILE "$_ => $vl->{$_}\n" for keys %$vl;
       }
      
     - while (my $c = $sf->get_next_case () )
     + while (my @c = $sf->get_next_case () )
       {
          for ($v = 0; $v < $dict->get_var_cnt(); $v++)
          {
     -  print MYFILE "val$v: \"@$c[$v]\"\n";
     +  print MYFILE "val$v: \"$c[$v]\"\n";
          }
          print MYFILE "\n";
       }
     @@ -469,9 +469,9 @@ EOF
      
       my $output = PSPP::Sysfile->new ("$tempdir/out.sav", $dict);
      
     - while (my $c = $input->get_next_case () )
     + while (my (@c) = $input->get_next_case () )
       {
     -   $output->append_case ($c);
     +   $output->append_case (address@hidden);
       }
      
       $output->close ();
     @@ -519,10 +519,10 @@ SYNTAX
      
       my $dict = $sf->get_dict ();
      
     - my $c = $sf->get_next_case ();
     + my (@c) = $sf->get_next_case ();
      
       my $var = $dict->get_var (0);
     - my $val = @$c[0];
     + my $val = $c[0];
       my $formatted = PSPP::format_value ($val, $var);
       my $str = gmtime ($val - PSPP::PERL_EPOCH);
       print "Formatted string is \"$formatted\"\n";
     @@ -557,30 +557,30 @@ SYNTAX
       my $dict = $sf->get_dict ();
      
      
     - my $c = $sf->get_next_case ();
     + my (@c) = $sf->get_next_case ();
      
       my $stringvar = $dict->get_var (0);
       my $numericvar = $dict->get_var (2);
     - my $val = @$c[0];
     + my $val = $c[0];
      
       ok ( !PSPP::value_is_missing ($val, $stringvar), "Missing Value Negative 
String");
      
     - $val = @$c[2];
     + $val = $c[2];
      
       ok ( !PSPP::value_is_missing ($val, $numericvar), "Missing Value 
Negative Num");
      
     - $c = $sf->get_next_case (); 
     - $c = $sf->get_next_case (); 
     + @c = $sf->get_next_case (); 
     + @c = $sf->get_next_case (); 
      
     - $val = @$c[0];
     + $val = $c[0];
       ok ( PSPP::value_is_missing ($val, $stringvar), "Missing Value 
Positive");
      
     - $c = $sf->get_next_case (); 
     - $val = @$c[2];
     + @c = $sf->get_next_case (); 
     + $val = $c[2];
       ok ( PSPP::value_is_missing ($val, $numericvar), "Missing Value Positive 
SYS");
      
     - $c = $sf->get_next_case (); 
     - $val = @$c[2];
     + @c = $sf->get_next_case (); 
     + $val = $c[2];
       ok ( PSPP::value_is_missing ($val, $numericvar), "Missing Value Positive 
Num");
      }
      

-- 
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://pgp.mit.edu or any PGP keyserver for public key.

signature.asc
Description: Digital signature