[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: ADD FILES and UPDATE
From: |
John Darrington |
Subject: |
Re: ADD FILES and UPDATE |
Date: |
Tue, 2 Dec 2008 14:42:12 +0900 |
User-agent: |
Mutt/1.5.13 (2006-08-11) |
On Mon, Dec 01, 2008 at 10:38:58AM -0500, Jason Stover wrote:
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?
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?
Sounds right to me.
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.
The compare_values function in src/data/value.c has the signature:
int compare_values (const union value *a, const union value *b,
const struct variable *var);
[... that's a little white lie, because the values are in fact all
of type void * so that the function can be passed to qsort etc. But
conceptually that's how the function should be used ]
So calling compare_values (v1, v2, var) is safe ONLY if BOTH v1 and v2
are values of var.
{
struct ccase c;
struct variable *var1;
struct variable *var2;
union value *v1, *v2, *v3;
v1 = case_data (&c, var1);
v2 = case_data (&c, var1);
v3 = case_data (&c, var2);
/* this is safe */
compare_values (v1, v2, var1);
/* this is not safe */
compare_values (v1, v3, var1);
}
It may be that we need a new function like
int
compare_values_extended (const union value *v1,
const struct variable *var1,
const union value *v2,
const struct variable *var2);
For the times when we wish to compare values which come from different
variables.
J'
--
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
- Re: ADD FILES and UPDATE, (continued)
- a better way to represent values (was: Re: ADD FILES and UPDATE), Ben Pfaff, 2008/12/08
- Re: a better way to represent values (was: Re: ADD FILES and UPDATE), John Darrington, 2008/12/08
- Re: a better way to represent values, Ben Pfaff, 2008/12/08
- Re: a better way to represent values, John Darrington, 2008/12/08
- Re: a better way to represent values, Ben Pfaff, 2008/12/08
- Re: a better way to represent values, John Darrington, 2008/12/09
- Re: a better way to represent values (was: Re: ADD FILES and UPDATE), Jason Stover, 2008/12/08
Re: ADD FILES and UPDATE, Jason Stover, 2008/12/01