NSHashTable and MapTable callbacks bugs

[Stephen Brandon]

Hi,

I'm pretty sure this is a bug in GNUstep - it seems at least to be a 
difference in implementation from that of MacOSX because code that works on 
MacOSX crashes GNUstep. And the documentation seems to describe the MacOSX 
behaviour rather than GNUstep.

Anyway, here's the problem:

When you create a set of NSMapTable callbacks, you are supposed to be able to 
set NULL for any one of them, and then that function is set to the default 
within Foundation (eg for hashing, equality, retaining, releasing, 
description).

What actually happens in gnustep-base is that the NULL pointer is called as a 
function, and segfaults.

Here's the flow of execution:
=======================

SETTING UP THE MAPTABLE:

NSMapTable.m:NSCreateMapTableWithZone(keyCallBacks,valueCallbacks,capacity,zone)

 - this creates a new map table by (eventually) getting to 
o_map.m:o_map_init_with_callbacks(), with the default set of callbacks 
(_NSMT_key_callbacks and _NSMT_value_callbacks). These callbacks are 
standardized in o_cbs.m:o_callbacks_standardize() by replacing any NULLs with 
legal, default callback functions (eg o_non_owned_void_p_hash). 
Unfortunately, these are not checking the user-supplied callbacks but the 
set of system defaults, so it does not help much. The sanitized set of 
callbacks get put into map->key_callbacks and map->value_callbacks.

 - the next stage in  NSMapTable.m:NSCreateMapTableWithZone is calling 
o_map_set_extra_callbacks(table, _NSMT_extra_callbacks) with the pre-defined 
set of callbacks (ok again so far)
 - it then calls o_map_set_extra(table, &extra) where the "extra" structure 
contains the user-defined callback structures.

These 2 functions are defined in o_map_bas.m (autogenerated):

o_map_set_extra_callbacks(map,callbacks) sets map->extra_callbacks to the 
pre-defined callbacks, then sets map->extra to 
(map->extra_callbacks).not_an_item_marker

o_map_set_extra(map,callbacks) puts the new set of user-defined callbacks 
into map->extra

At this stage we now have a new map table with the following:

map->key_callbacks contains a sanitized copy of _NSMT_key_callbacks
map->value_callbacks contains a sanitized copy of _NSMT_value_callbacks
map->extra_callbacks contains a copy of _NSMT_extra_callbacks
map->extra contains the set of user callbacks (still containing NULLs).

USING THE MAPTABLE

In the MusicKit, I call the following function to do a lookup on a previously 
created map:

mapNode = (midiOutNode *) NSMapGet(ptr->_map[chan], (const void *) noteTag);

This calls o_map_value_at_key(table, key)

That eventually calls _o_map_pick_bucket_for_key which utilizes for the first 
time map->key_callbacks during o_hash. o_hash does the right thing and 
double-checks the key_callbacks.hash function for nullness before calling it 
(good, but that's not our user-defined ones).

Unfortunately, map->key_callbacks.hash = _NSMT_key_hash (defined in 
NSMapTable.m). This function eventually calls one of my null callback 
functions without checking it:

_NSMT_key_hash uses a macro which expands thus:
NSMT_KEY_CALLBACKS(table).hash expands to 
((NSMT_EXTRA(table)->keyCallBacks).hash expands to (_NSMT_extra_t 
*)(o_map_extra((o_map_t *)table)).hash

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
o_map_extra(XX) equates to XX->extra, so we're looking at table->extra.hash, 
which we see from above is NULL, and it is called but never checked.
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

FIX
===
Unless there's a better way of doing this, could I suggest that the 
_NSMT_key_hash, _NSMT_key_compare, _NSMT_key_is_equal, _NSMT_key_retain, 
_NSMT_key_release, _NSMT_value_retain, and _NSMT_value_release functions in 
NSMapTable.m all check for nullness before calling their user-defined 
functions?

What I don't know though is exactly which finctions should be called if the 
user-defined ones are NULL, as there seem to be a number to choose from... 
(perhaps the _NSMT_extra_callbacks as they are fleshed out in NSMapTable.m???)

Any help from the experts appreciated!

Cheers,
Stephen Brandon
stephen@brandonitconsulting.co.uk