Re: ADD FILES and UPDATE

[Jason Stover]

I'm a little confused here. My confused comments are below.

On Sun, Nov 30, 2008 at 09:00:56PM -0800, Ben Pfaff wrote:
> John Darrington <address@hidden> writes:
> 
> > I also wasn't aware that compare_values only checked the first 8
> > characters.  I have a feeling this is a bug.
> 
> Yes, in many cases I think so too.

The bug being that if someone passes a union value with more than 8
characters, only the first 8 are compared. So, for example,
compare_values will return 0 when comparing 'aaaaaaaa' and
'aaaaaaaab', when it should return 1 or -1. Right?

> 
> Here is my analysis of each use in the source tree:
...
> 
>         * src/math/coefficient.c (pspp_coeff_var_to_coeff): I
>           don't know.  Further investigation required.
> 
>         * src/math/covariance-matrix.c (various functions):
>           Ditto.

As far as I can tell, the functions in both of these files will show
the bug. Neither they nor functions that call them check the number of
characters in the union values in their arguments. But I'm not sure
why they would cause a problem if this one doesn't:

>         * src/data/category.c (cat_value_find): The code assumes
>           that each candidate is exactly 1 union value in size,
>           so I don't think that this is a bug here.

This function doesn't check the number of characters in the union
values either.  So comparing 'aaaaaaaa' and 'aaaaaaaab' will return 0
when it shouldn't.  Right?

> > The only problem with the 3way function is that the caller must
> > gaurantee that both values have the same with.
> >
> > I think that we need a more general compare_values_with_widths_3way
> > function and all existing callers of compare_values_short should use it.
> 
> That sounds like a good idea.  I know that in some places
> comparison of strings of differing length is done by extending
> the shorter string with spaces to the length of the longer
> string, then comparing the result; see, e.g., compare_string() in
> src/language/expressions/helpers.c, which is used for string
> inequality comparison operators (and elsewhere) in expressions.
> Are you aware of unequal-length strings being compared any other
> way in PSPP?

As a naive user of compare_values, I had assumed I could call it to
compare values, and not have to worry about how many characters were
in the values. Now I'm thinking I need to learn otherwise.  So how
should I be using compare_values? Should I add my own code to check
the number of characters in the union value every time I want to
compare values?

I'll need to compare a lot of values in the code to handle
interactions, so it would be good to have something whose guts I don't
need to know about. compare_values (val1, val2) is easy.

-Jason