John,

I think you missed my main point, which is that your new code *wont
work* because it never assigns the correct variable.  More details
inline...

John Ralls <[email protected]> writes:

[snip]
>>> -    gnc_numeric *key;
>>> +    gnc_numeric *key = NULL;
>>> +   gpointer pkey = (gpointer)key; 

Okay, so key is now NULL, which is fine.  But then the second line
just assigns pkey to the VALUE of key, which means pkey == NULL.

[snip]
>>> -    while (g_hash_table_iter_next (&iter, (gpointer *)&key, NULL))
>>> +    while (g_hash_table_iter_next (&iter, &pkey, NULL))

In this call, g_hash_table_iter_next() will assign the next value into
pkey.  So now pkey has the new item, but key is STILL NULL.

>>>       /* Compute a new reachable value */
>>>       gnc_numeric reachable_value = gnc_numeric_add_fixed(*key, 
>>> split_value);

And now you try to dereference key, which is still NULL.  *boom*
Null-pointer dereference.

[snip]
>> Same problem here!   psplit will get set, but that wont affect the value
>> of split.  The original code looks just fine to me!

And you do the same thing in the second portion, too.  You'll get a NULL
pointer dereference once you try to access 'split'

> The problem in both cases was that gcc-4.1.3 puked on casting the address of 
> key to a gpointer*:
> cc1: warnings being treated as errors
> window-autoclear.c: In function ‘gnc_autoclear_window_ok_cb’:
> window-autoclear.c:171: warning: dereferencing type-punned pointer will break 
> strict-aliasing rules
> (And, of course, the same for split.)

Fair enough.  I wonder if we actually NEED the explicit cast?
If we do then we need to come up with another way of doing it,
maybe by re-assigning after the fill-in call.

> Note that I haven't changed the level of indirection passed to 
> g_hash_table_foo; it's effectively a void** (because a gpointer is a typedef 
> of void *) in both the old and new -- which is what g_hash_table_iter_next 
> and g_hash_table_lookup_extended expect, so presumably they need to 
> double-dereference.

No, you haven't.  However you also haven't re-assigned the original
variable, either, which means you'll get a NULL pointer exception.

> Regards,
> John Ralls

-derek

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       [email protected]                        PGP key available
_______________________________________________
gnucash-devel mailing list
[email protected]
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Reply via email to