[patch #5636] Add callback feature to dictionary

[Ben Pfaff]

Follow-up Comment #1, patch #5636 (project pspp):

This diff is a little hard to read because of several "changes" that appear
to just change invisible white space.  It looks OK though.  Much cleaner GUI
code in places.

The assertions in var_set_decimals are wrong.  I'd recommend simply setting
the decimal places, then asserting on fmt_check_output.

This change in var_set_width should be unnecessary:
-          v->val_labs = NULL;
+          v->val_labs = val_labs_create (new_width);
If you put it in thinking that it would always make ->val_labs non-null, it
is insufficient, because var_create sets ->val_labs to null.  But I don't
think any code depends on that.

In src/ui/gui/missing-val-dialog.c:
-                          psppire_variable_get_type(dialog->pv) ==
VAR_NUMERIC)
;
+                          var_get_type(dialog->pv) == VAR_NUMERIC);
You could use var_is_numeric(dialog->pv) here.

Similarly for VAR_STRING ==  var_get_type (pv) in
src/ui/gui/psppire-data-store.c.

The psppire_var_store_set_string function looks suspicious.  I don't know
where its input is coming from, but it looks unsafe if it's not coming from
someplace that's been validated, e.g. does the following change work? 
var_set_name should assert-fail if the variable is in a dictionary;
dict_rename_var is supposed to be used instead:
     case COL_NAME:
-      return psppire_variable_set_name(pv, text);
+      var_set_name (pv, text);
+      return TRUE;
       break;

Similarly, this is going to assert-fail if the decimals are not in the
allowed range:
     case COL_DECIMALS:
       if ( ! text) return FALSE;
-      return psppire_variable_set_decimals(pv, atoi(text));
+      var_set_decimals (pv, atoi(text));
+      return TRUE;
       break;

The following check could be simplified to var_has_value_labels, which
internally does the same thing:
+       if ( ! vls || 0 == val_labs_count (vls) ) 



    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?5636>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/