Re: suspicious code in convert_xml_string_to_value()

[Ben Pfaff]

John Darrington <address@hidden> writes:

> I think the correct behaviour when text is NULL, is to 
> set v->f to SYSMIS in the case of numeric variables, 
> and to the empty string for string variables.

Thanks.  I am going to push the following, if it passes "make
check".

commit a75daca6a9423892954da6ddcecb9d518b24c024
Author: Ben Pfaff <address@hidden>
Date:   Sat Feb 20 11:24:34 2010 -0800

    gnumeric-reader: Avoid convert_xml_string_to_value segfault on NULL string
    
    If the 'xv' argument is null, the previous version of this code would
    still dereference it.  This commit rewrites it to instead just set the
    value as missing and avoid the segfault.  It also takes advantage of the
    fairly recently added function value_copy_str_rpad() to eliminate some
    code.

diff --git a/src/data/gnumeric-reader.c b/src/data/gnumeric-reader.c
index f166ee6..74911c4 100644
--- a/src/data/gnumeric-reader.c
+++ b/src/data/gnumeric-reader.c
@@ -1,5 +1,5 @@
 /* PSPP - a program for statistical analysis.
-   Copyright (C) 2007, 2009 Free Software Foundation, Inc.
+   Copyright (C) 2007, 2009, 2010 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -320,21 +320,17 @@ static void
 convert_xml_string_to_value (struct ccase *c, const struct variable *var,
                             const xmlChar *xv)
 {
-  int n_bytes = 0;
   union value *v = case_data_rw (c, var);
 
-  const char *text = (const char *) xv;
-
-  if ( text)
-    n_bytes = MIN (var_get_width (var), strlen (text));
-
-  if ( var_is_alpha (var))
-    {
-      memcpy (value_str_rw (v, var_get_width (var)), text, n_bytes);
-    }
+  if (xv == NULL)
+    value_set_missing (v, var_get_width (var));
+  else if ( var_is_alpha (var))
+    value_copy_str_rpad (v, var_get_width (var), xv, ' ');
   else
     {
+      const char *text = (const char *) xv;
       char *endptr;
+
       errno = 0;
       v->f = strtod (text, &endptr);
       if ( errno != 0 || endptr == text)

-- 
Ben Pfaff 
http://benpfaff.org