hash/map table and pointer functions fixups

This commit is contained in:
rfm 2024-11-12 12:03:24 +00:00
parent c981920679
commit b78b2e2030
8 changed files with 290 additions and 89 deletions

View file

@ -1,3 +1,30 @@
2024-11-12 Richard Frith-Macdonald <rfm@gnu.org>
* Headers/GNUstepBase/GSIMap.h:
Change (and document) the 'clear' macros for removing keys and values
from map tables so they can be used properly. Change 'write' macros
to be 'store' macros, reflecting a roll of storing vvalues into the
map. Standardise macro names referring to values to end in 'VALUE'.
Tidy up and improve comments.
* Source/NSConcreteHashTable.m:
Replace macros with retain/release macros which do nothing and
store/clear macros which invoke the appropriate functions.
* Source/NSConcreteMapTable.m:
Replace macros with retain/release macros which do nothing and
store/clear macros which invoke the appropriate functions.
* Source/NSConcretePointerFunctions.h:
In pointerFunctionsReplace() fix bug setting the new value to zero
thather than the value supplied.
* Source/NSConcretePointerFunctions.m:
Set correct relinquish function for
NSPointerFunctionsObjectPointerPersonality
* Tests/base/NSHashTable/general.m:
Add tests for NSHashTableObjectPointerPersonality and
NSObjectHashCallBacks
* Tests/base/NSMapTable/general.m:
Add tests for NSMapTableObjectPointerPersonality and
NSObjectMapKeyCallBacks/NSObjectMapValueCallBacks
2024-11-09 Richard Frith-Macdonald <rfm@gnu.org>
* Source/NSNotificationCenter.m:

View file

@ -66,23 +66,43 @@ extern "C" {
* If defined as 0, then this becomes a hash table rather than
* a map table.
*
* The retain/release macros are a historical legacy to support memory
* management using explicit retain and release. They may be defined
* to be a no-op with all the work being done by store/clear macros.
*
* GSI_MAP_RETAIN_KEY()
* Macro to retain the key item in a map or hash table.
*
* GSI_MAP_RETAIN_VAL()
* GSI_MAP_RETAIN_VALUE()
* Macro to retain the value item in a map table.
*
* GSI_MAP_RELEASE_KEY()
* Macro to release the key item in a map or hash table.
*
* GSI_MAP_RELEASE_VAL()
* GSI_MAP_RELEASE_VALUE()
* Macro to release the value item in a map table.
*
* GSI_MAP_WRITE_KEY()
* Macro defining the write barrier for a key.
*
* GSI_MAP_WRITE_VAL()
* Macro defining the write barrier for a value.
* The store/clear macros are used to store values into the map table
* and to zero-out values removed from the map table.
* Where the retain/release macros do nothing, the store/clear macros
* should perform memory management operations as well as copying.
*
* GSI_MAP_STORE_KEY()
* Macro for storing a key into the map table and
* defining the write barrier for that key in the table.
*
* GSI_MAP_STORE_VALUE()
* Macro for storing a value into the map table and
* defining the write barrier for that value in the table.
*
* GSI_MAP_CLEAR_KEY()
* Macro for removing a key from the map table and zeroing
* out the memory location.
*
* GSI_MAP_CLEAR_VALUE()
* Macro for removing a value from the map table and zeroing
* out the memory location.
*
* GSI_MAP_READ_KEY()
* Macro defining the read barrier for a key.
@ -124,12 +144,20 @@ extern "C" {
#ifndef GSI_MAP_RELEASE_KEY
#define GSI_MAP_RELEASE_KEY(M, X) [(X).obj release]
#endif
#ifndef GSI_MAP_RETAIN_VAL
#define GSI_MAP_RETAIN_VAL(M, X) [(X).obj retain]
#if !defined(GSI_MAP_RETAIN_VALUE)
#if defined(GSI_MAP_RETAIN_VAL)
#define GSI_MAP_RETAIN_VALUE(M, X) GSI_MAP_RETAIN_VAL(M, X)
#else
#define GSI_MAP_RETAIN_VALUE(M, X) [(X).obj retain]
#endif
#ifndef GSI_MAP_RELEASE_VAL
#define GSI_MAP_RELEASE_VAL(M, X) [(X).obj release]
#endif // !defined(GSI_MAP_RETAIN_VALUE)
#if !defined(GSI_MAP_RELEASE_VALUE)
#if defined(GSI_MAP_RELEASE_VAL)
#define GSI_MAP_RELEASE_VALUE(M, X) GSI_MAP_RELEASE_VAL(M, X)
#else
#define GSI_MAP_RELEASE_VALUE(M, X) [(X).obj release]
#endif
#endif // !defined(GSI_MAP_RELEASE_VALUE)
#ifndef GSI_MAP_HASH
#define GSI_MAP_HASH(M, X) [(X).obj hash]
#endif
@ -149,11 +177,13 @@ extern "C" {
#ifndef GSI_MAP_READ_VALUE
# define GSI_MAP_READ_VALUE(M, x) (*(x))
#endif
#ifndef GSI_MAP_WRITE_KEY
# define GSI_MAP_WRITE_KEY(M, addr, obj) (*(addr) = obj)
#ifndef GSI_MAP_STORE_KEY
# define GSI_MAP_STORE_KEY(M, addr, obj)\
*(addr) = obj
#endif
#ifndef GSI_MAP_WRITE_VAL
# define GSI_MAP_WRITE_VAL(M, addr, obj) (*(addr) = obj)
#ifndef GSI_MAP_STORE_VALUE
# define GSI_MAP_STORE_VALUE(M, addr, obj)\
*(addr) = obj
#endif
#if GSI_MAP_HAS_VALUE
#define GSI_MAP_NODE_IS_EMPTY(M, node) \
@ -205,12 +235,16 @@ extern "C" {
#endif
#if !defined(GSI_MAP_CLEAR_KEY)
#if (GSI_MAP_KTYPES) & GSUNION_OBJ
#define GSI_MAP_CLEAR_KEY(node) GSI_MAP_WRITE_KEY(map, &node->key, (GSIMapKey)(id)nil)
#define GSI_MAP_CLEAR_KEY(map, addr)\
GSI_MAP_STORE_KEY(map, addr, (GSIMapKey)(id)nil)
#elif (GSI_MAP_KTYPES) & GSUNION_PTR
#define GSI_MAP_CLEAR_KEY(node) GSI_MAP_WRITE_KEY(map, &node->key, (GSIMapKey)(void *)NULL)
#define GSI_MAP_CLEAR_KEY(map, addr)\
GSI_MAP_STORE_KEY(map, addr, (GSIMapKey)(void *)NULL)
#else
#define GSI_MAP_CLEAR_KEY(node)
#define GSI_MAP_CLEAR_KEY(map, addr)
#endif
#endif
/*
@ -257,12 +291,16 @@ extern "C" {
#include <GNUstepBase/GSUnion.h>
#endif
#if !defined(GSI_MAP_CLEAR_VALUE)
#if (GSI_MAP_VTYPES) & GSUNION_OBJ
#define GSI_MAP_CLEAR_VAL(node) GSI_MAP_WRITE_VAL(map, &node->value, (GSIMapVal)(id)nil)
#define GSI_MAP_CLEAR_VALUE(map, addr)\
GSI_MAP_STORE_VALUE(map, addr, (GSIMapVal)(id)nil)
#elif (GSI_MAP_VTYPES) & GSUNION_PTR
#define GSI_MAP_CLEAR_VAL(node) GSI_MAP_WRITE_VAL(map, &node->value, (GSIMapVal)(void *)NULL)
#define GSI_MAP_CLEAR_VALUE(map, addr)\
GSI_MAP_STORE_VALUE(map, addr, (GSIMapVal)(void *)NULL)
#else
#define GSI_MAP_CLEAR_VAL(node)
#define GSI_MAP_CLEAR_VALUE(map, addr)
#endif
#endif
/*
@ -472,12 +510,11 @@ GS_STATIC_INLINE void
GSIMapFreeNode(GSIMapTable map, GSIMapNode node)
{
GSI_MAP_RELEASE_KEY(map, node->key);
GSI_MAP_CLEAR_KEY(node);
GSI_MAP_CLEAR_KEY(map, &node->key);
#if GSI_MAP_HAS_VALUE
GSI_MAP_RELEASE_VAL(map, node->value);
GSI_MAP_CLEAR_VAL(node);
GSI_MAP_RELEASE_VALUE(map, node->value);
GSI_MAP_CLEAR_VALUE(map, &node->value);
#endif
node->nextInBucket = map->freeNodes;
map->freeNodes = node;
}
@ -1053,8 +1090,10 @@ GSIMapAddPairNoRetain(GSIMapTable map, GSIMapKey key, GSIMapVal value)
node = map->freeNodes;
}
map->freeNodes = node->nextInBucket;
GSI_MAP_WRITE_KEY(map, &node->key, key);
GSI_MAP_WRITE_VAL(map, &node->value, value);
node->key = key;
node->value = value;
GSI_MAP_STORE_KEY(map, &node->key, key);
GSI_MAP_STORE_VALUE(map, &node->value, value);
node->nextInBucket = 0;
GSIMapRightSizeMap(map, map->nodeCount);
GSIMapAddNodeToMap(map, node);
@ -1072,10 +1111,10 @@ GSIMapAddPair(GSIMapTable map, GSIMapKey key, GSIMapVal value)
node = map->freeNodes;
}
map->freeNodes = node->nextInBucket;
GSI_MAP_WRITE_KEY(map, &node->key, key);
GSI_MAP_STORE_KEY(map, &node->key, key);
GSI_MAP_RETAIN_KEY(map, node->key);
GSI_MAP_WRITE_VAL(map, &node->value, value);
GSI_MAP_RETAIN_VAL(map, node->value);
GSI_MAP_STORE_VALUE(map, &node->value, value);
GSI_MAP_RETAIN_VALUE(map, node->value);
node->nextInBucket = 0;
GSIMapRightSizeMap(map, map->nodeCount);
GSIMapAddNodeToMap(map, node);
@ -1093,7 +1132,7 @@ GSIMapAddKeyNoRetain(GSIMapTable map, GSIMapKey key)
node = map->freeNodes;
}
map->freeNodes = node->nextInBucket;
GSI_MAP_WRITE_KEY(map, &node->key, key);
GSI_MAP_STORE_KEY(map, &node->key, key);
node->nextInBucket = 0;
GSIMapRightSizeMap(map, map->nodeCount);
GSIMapAddNodeToMap(map, node);
@ -1111,7 +1150,7 @@ GSIMapAddKey(GSIMapTable map, GSIMapKey key)
node = map->freeNodes;
}
map->freeNodes = node->nextInBucket;
GSI_MAP_WRITE_KEY(map, &node->key, key);
GSI_MAP_STORE_KEY(map, &node->key, key);
GSI_MAP_RETAIN_KEY(map, node->key);
node->nextInBucket = 0;
GSIMapRightSizeMap(map, map->nodeCount);
@ -1166,11 +1205,10 @@ GSIMapCleanMap(GSIMapTable map)
while(node != 0)
{
GSI_MAP_RELEASE_KEY(map, node->key);
GSI_MAP_CLEAR_KEY(node);
GSI_MAP_CLEAR_KEY(map, &node->key);
#if GSI_MAP_HAS_VALUE
GSI_MAP_RELEASE_VAL(map, node->value);
GSI_MAP_CLEAR_VAL(node);
GSI_MAP_RELEASE_VALUE(map, node->value);
GSI_MAP_CLEAR_VALUE(map, &node->value);
#endif
prevNode = node;
node = node->nextInBucket;

View file

@ -83,22 +83,27 @@ typedef GSIMapNode_t *GSIMapNode;
#define GSI_MAP_EQUAL(M, X, Y)\
(M->legacy ? M->cb.old.isEqual(M, X.ptr, Y.ptr) \
: pointerFunctionsEqual(&M->cb.pf, X.ptr, Y.ptr))
#define GSI_MAP_RELEASE_KEY(M, X)\
(M->legacy ? M->cb.old.release(M, X.ptr) \
: IS_WEAK(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf, &X.ptr))
#define GSI_MAP_RETAIN_KEY(M, X)\
(M->legacy ? M->cb.old.retain(M, X.ptr) \
: IS_WEAK(M) ? nil : pointerFunctionsAssign(\
&M->cb.pf, &X.ptr, pointerFunctionsAcquire(&M->cb.pf, X.ptr)))
#define GSI_MAP_ZEROED(M)\
(M->legacy ? 0 \
: (IS_WEAK(M) ? YES : NO))
(M->legacy ? 0 : (IS_WEAK(M) ? YES : NO))
/* NSPointerFunctions provides functions which combine the actions of
* memory allocation/deallocation with those of assignment, so we make
* the separete retain/release macros a no-op nd do all the work in the
* store/clear macros.
*/
#define GSI_MAP_RELEASE_KEY(M, X)
#define GSI_MAP_RETAIN_KEY(M, X) nil
#define GSI_MAP_CLEAR_KEY(M, addr)\
if (M->legacy) \
{ M->cb.old.release(M, (*addr).ptr); (*addr).ptr = 0; }\
else\
pointerFunctionsRelinquish(&M->cb.pf, (void**)addr);
#define GSI_MAP_STORE_KEY(M, addr, x)\
if (M->legacy)\
{ *(addr) = x; M->cb.old.retain(M, (*addr).ptr); }\
else\
pointerFunctionsReplace(&M->cb.pf, (void**)addr, (x).obj);
#define GSI_MAP_WRITE_KEY(M, addr, x) \
if (M->legacy) \
*(addr) = x;\
else\
pointerFunctionsAssign(&M->cb.pf, (void**)addr, (x).obj);
#define GSI_MAP_READ_KEY(M,addr) \
(M->legacy ? *(addr) :\
(__typeof__(*addr))pointerFunctionsRead(&M->cb.pf, (void**)addr))

View file

@ -90,38 +90,39 @@ typedef GSIMapNode_t *GSIMapNode;
#define GSI_MAP_EQUAL(M, X, Y)\
(M->legacy ? M->cb.old.k.isEqual(M, X.ptr, Y.ptr) \
: pointerFunctionsEqual(&M->cb.pf.k, X.ptr, Y.ptr))
#define GSI_MAP_RELEASE_KEY(M, X)\
(M->legacy ? M->cb.old.k.release(M, X.ptr) \
: IS_WEAK_KEY(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf.k, &X.ptr))
#define GSI_MAP_RETAIN_KEY(M, X)\
(M->legacy ? M->cb.old.k.retain(M, X.ptr) \
: IS_WEAK_KEY(M) ? nil : pointerFunctionsAssign(&M->cb.pf.k, &X.ptr,\
pointerFunctionsAcquire(&M->cb.pf.k, X.ptr)))
#define GSI_MAP_RELEASE_VAL(M, X)\
(M->legacy ? M->cb.old.v.release(M, X.ptr) \
: IS_WEAK_VALUE(M) ? nil : pointerFunctionsRelinquish(&M->cb.pf.v, &X.ptr))
#define GSI_MAP_RETAIN_VAL(M, X)\
(M->legacy ? M->cb.old.v.retain(M, X.ptr) \
: IS_WEAK_VALUE(M) ? nil : pointerFunctionsAssign(&M->cb.pf.v, &X.ptr,\
pointerFunctionsAcquire(&M->cb.pf.v, X.ptr)))
/* The GSI_MAP_WRITE_KEY() and GSI_MAP_WRITE_VAL() macros are expectd to
* write without retain (GSI_MAP RETAIN macros are executed separately)
* so they can't use pointerFunctionsAssign() unless working with weak
* references.
/* NSPointerFunctions provides functions which combine the actions of
* memory allocation/deallocation with those of assignment, so we make
* the separete retain/release macros a no-op nd do all the work in the
* store/clear macros.
*/
#define GSI_MAP_WRITE_KEY(M, addr, x) \
#define GSI_MAP_RELEASE_KEY(M, X)
#define GSI_MAP_RETAIN_KEY(M, X) nil
#define GSI_MAP_CLEAR_KEY(M, addr)\
if (M->legacy) \
*(addr) = x;\
{ M->cb.old.k.release(M, (*addr).ptr); (*addr).ptr = 0; }\
else\
(IS_WEAK_KEY(M) ? pointerFunctionsAssign(&M->cb.pf.k, (void**)addr,\
(x).obj) : (*(id*)(addr) = (x).obj));
#define GSI_MAP_WRITE_VAL(M, addr, x) \
pointerFunctionsRelinquish(&M->cb.pf.k, (void**)addr);
#define GSI_MAP_STORE_KEY(M, addr, x)\
if (M->legacy)\
{ *(addr) = x; M->cb.old.k.retain(M, (*addr).ptr); }\
else\
pointerFunctionsReplace(&M->cb.pf.k, (void**)addr, (x).obj);
#define GSI_MAP_RELEASE_VALUE(M, X)
#define GSI_MAP_RETAIN_VALUE(M, X) nil
#define GSI_MAP_CLEAR_VALUE(M, addr)\
if (M->legacy) \
*(addr) = x;\
{ M->cb.old.v.release(M, (*addr).ptr); (*addr).ptr = 0; }\
else\
(IS_WEAK_VALUE(M) ? pointerFunctionsAssign(&M->cb.pf.v, (void**)addr,\
(x).obj) : (*(id*)(addr) = (x).obj));
pointerFunctionsRelinquish(&M->cb.pf.v, (void**)addr);
#define GSI_MAP_STORE_VALUE(M, addr, x)\
if (M->legacy)\
{ *(addr) = x; M->cb.old.v.retain(M, (*addr).ptr); }\
else\
pointerFunctionsReplace(&M->cb.pf.v, (void**)addr, (x).obj);
#define GSI_MAP_READ_KEY(M,addr) \
(M->legacy ? *(addr)\
: (__typeof__(*addr))pointerFunctionsRead(&M->cb.pf.k, (void**)addr))
@ -638,13 +639,9 @@ NSMapInsert(NSMapTable *table, const void *key, const void *value)
GSIMapAddPair(t, (GSIMapKey)key, (GSIMapVal)value);
t->version++;
}
else if (n->value.ptr != value)
else if (GSI_MAP_READ_VALUE(t, &n->value).ptr != value)
{
GSIMapVal tmp = n->value;
n->value = (GSIMapVal)value;
GSI_MAP_RETAIN_VAL(t, n->value);
GSI_MAP_RELEASE_VAL(t, tmp);
GSI_MAP_STORE_VALUE(t, &n->value, (GSIMapVal)value);
t->version++;
}
}
@ -1383,9 +1380,7 @@ const NSMapTableValueCallBacks NSOwnedPointerMapValueCallBacks =
{
if (GSI_MAP_READ_VALUE(self, &node->value).obj != anObject)
{
GSI_MAP_RELEASE_VAL(self, node->value);
GSI_MAP_WRITE_VAL(self, &node->value, (GSIMapVal)anObject);
GSI_MAP_RETAIN_VAL(self, node->value);
GSI_MAP_STORE_VALUE(self, &node->value, (GSIMapVal)anObject);
version++;
}
}

View file

@ -182,7 +182,6 @@ pointerFunctionsEqual(PFInfo *PF, void *item1, void *item2)
static inline void
pointerFunctionsRelinquish(PFInfo *PF, void **itemptr)
{
if (PF->relinquishFunction != 0)
(*PF->relinquishFunction)(*itemptr, PF->sizeFunction);
if (memoryType(PF->options, NSPointerFunctionsWeakMemory))
@ -203,7 +202,7 @@ pointerFunctionsReplace(PFInfo *PF, void **dst, void *src)
if (PF->relinquishFunction != 0)
(*PF->relinquishFunction)(*dst, PF->sizeFunction);
if (memoryType(PF->options, NSPointerFunctionsWeakMemory))
WEAK_WRITE(dst, 0);
WEAK_WRITE(dst, src);
else
*dst = src;
}

View file

@ -300,7 +300,7 @@ relinquishRetainedMemory(const void *item,
else
{
_x.acquireFunction = acquireRetainedObject;
_x.relinquishFunction = 0;
_x.relinquishFunction = relinquishRetainedMemory;
}
_x.descriptionFunction = describeObject;
_x.hashFunction = hashShifted;

View file

@ -1,9 +1,70 @@
#import <Foundation/NSAutoreleasePool.h>
#import <Foundation/NSHashTable.h>
#import "ObjectTesting.h"
@interface MyClass: NSObject
@end
@implementation MyClass
#if 0
- (oneway void) release
{
NSLog(@"releasing %u", (unsigned)[self retainCount]);
[super release];
}
- (id) retain
{
id result = [super retain];
NSLog(@"retained %u", (unsigned)[self retainCount]);
return result;
}
#endif
@end
int main()
{
NSAutoreleasePool *arp = [NSAutoreleasePool new];
NSAutoreleasePool *arp = [NSAutoreleasePool new];
NSHashTable *t;
MyClass *o;
int c;
t = [[NSHashTable alloc] initWithOptions: NSHashTableObjectPointerPersonality
capacity: 10];
o = [MyClass new];
c = [o retainCount];
PASS(c == 1, "initial retain count is one")
[t addObject: @"a"];
[t addObject: o];
PASS([o retainCount] == c + 1, "add to hash table increments retain count")
PASS(NSHashGet(t, o) == o, "object found in table")
[t removeObject: o];
PASS([o retainCount] == c, "remove from hash table decrements retain count")
RELEASE(t);
RELEASE(o);
t = NSCreateHashTable(NSObjectHashCallBacks, 10);
o = [MyClass new];
c = [o retainCount];
PASS(c == 1, "initial retain count is one")
[t addObject: @"a"];
[t addObject: o];
PASS([o retainCount] == c + 1, "add to hash table increments retain count")
PASS(NSHashGet(t, o) == o, "object found in table")
[t removeObject: o];
PASS([o retainCount] == c, "remove from hash table decrements retain count")
RELEASE(t);
RELEASE(o);
[arp release]; arp = nil;
return 0;

View file

@ -1,9 +1,85 @@
#import <Foundation/NSAutoreleasePool.h>
#import "ObjectTesting.h"
#import <Foundation/NSMapTable.h>
@interface MyClass: NSObject
@end
@implementation MyClass
#if 0
- (oneway void) release
{
NSLog(@"releasing %u", (unsigned)[self retainCount]);
[super release];
}
- (id) retain
{
id result = [super retain];
NSLog(@"retained %u", (unsigned)[self retainCount]);
return result;
}
#endif
@end
int main()
{
NSAutoreleasePool *arp = [NSAutoreleasePool new];
NSAutoreleasePool *arp = [NSAutoreleasePool new];
NSMapTable *t;
MyClass *o;
int c;
t = [[NSMapTable alloc] initWithKeyOptions: NSMapTableObjectPointerPersonality
valueOptions: NSMapTableObjectPointerPersonality
capacity: 10];
o = [MyClass new];
c = [o retainCount];
PASS(c == 1, "initial retain count is one")
[t setObject: @"a" forKey: o];
PASS([o retainCount] == c + 1, "add map table key increments retain count")
PASS_EQUAL((id)NSMapGet(t, o), @"a", "object found in table")
[t removeObjectForKey: o];
PASS([o retainCount] == c, "remove map table key decrements retain count")
[t setObject: o forKey: @"a"];
PASS([o retainCount] == c + 1, "add map table val increments retain count")
PASS_EQUAL((id)NSMapGet(t, @"a"), o, "object found in table")
[t removeObjectForKey: @"a"];
PASS([o retainCount] == c, "remove map table val decrements retain count")
RELEASE(t);
RELEASE(o);
t = NSCreateMapTable(NSObjectMapKeyCallBacks, NSObjectMapValueCallBacks, 10);
o = [MyClass new];
c = [o retainCount];
PASS(c == 1, "initial retain count is one")
[t setObject: @"a" forKey: o];
PASS([o retainCount] == c + 1, "add map table key increments retain count")
PASS_EQUAL((id)NSMapGet(t, o), @"a", "object found in table")
[t removeObjectForKey: o];
PASS([o retainCount] == c, "remove map table key decrements retain count")
[t setObject: o forKey: @"a"];
PASS([o retainCount] == c + 1, "add map table val increments retain count")
PASS_EQUAL((id)NSMapGet(t, @"a"), o, "object found in table")
[t removeObjectForKey: @"a"];
PASS([o retainCount] == c, "remove map table val decrements retain count")
RELEASE(t);
RELEASE(o);
[arp release]; arp = nil;
return 0;