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

[John Ralls]

On Feb 8, 2010, at 9:34 AM, Derek Atkins wrote:

> Hi,
> 
> John Ralls <jra...@code.gnucash.org> writes:
> 
>> -    gnc_numeric *key;
>> +    gnc_numeric *key = NULL;
>> +    gpointer pkey = (gpointer)key; 
>>     GList *reachable_list = 0, *node;
>> 
>>     printf("  Split value: %s\n", gnc_numeric_to_string(split_value));
>> 
>>     /* For each value in the sack */
>>     g_hash_table_iter_init (&iter, sack);
>> -    while (g_hash_table_iter_next (&iter, (gpointer *)&key, NULL))
>> +    while (g_hash_table_iter_next (&iter, &pkey, NULL))
>>     {
>>       /* Compute a new reachable value */
>>       gnc_numeric reachable_value = gnc_numeric_add_fixed(*key, split_value);
> 
> I don't think this is going to work right..  the g_hash_table_iter_next()
> will set pkey, but that wont change the value of key.  So the
> gnc_numeric_add_fixed() will fail because *key will still be NULL.
> 
> What was wrong with the original code?
> 
>> @@ -203,10 +204,11 @@
>>   printf("Rebuilding solution ...\n");
>>   while (!gnc_numeric_zero_p(toclear_value))
>>   {
>> -    Split *split;
>> +    Split *split = NULL;
>> +    gpointer psplit = (gpointer)split;
>> 
>>     printf("  Left to clear: %s\n", gnc_numeric_to_string(toclear_value));
>> -    if (g_hash_table_lookup_extended(sack, &toclear_value, NULL, (gpointer 
>> *)&split))
>> +    if (g_hash_table_lookup_extended(sack, &toclear_value, NULL, &psplit))
>>     {
>>       if (split != NULL)
> 
> Same problem here!   psplit will get set, but that wont affect the value
> of split.  The original code looks just fine to me!

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

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.

Regards,
John Ralls



_______________________________________________
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel