r18612 - gnucash/trunk/src - Corrections to r18610 to permit compiling against older versions of glib

Derek Atkins warlord at MIT.EDU
Tue Feb 9 08:54:12 EST 2010


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

John Ralls <jralls at ceridwen.us> writes:

>>> -    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.

>>> -    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.

>> 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 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
       warlord at MIT.EDU                        PGP key available

More information about the gnucash-devel mailing list