...stinks.

I'm talking about sqAllocateMemory.  Here's the base definition from platforms/Cross/vm/sq.h:

#define sqAllocateMemory(minHeapSize, desiredHeapSize)  malloc(desiredHeapSize)

The problem here is that there's no obvious way for the client to know how much memory is returned.  The signature implies it is somewhere between minHeapSize & desiredHeapSize inclusive (or null, if allocation failed).  But how do you tell?  Well, one could always ask to grow memory by 0 and see what you get back, except for the small chicken-and-egg problem that sqGrowMemory:By: and sqShrinkMemory:By: require the ammount of memory allocated as an argument:

#define sqGrowMemoryBy(oldLimit, delta)         oldLimit
#define sqShrinkMemoryBy(oldLimit, delta)       oldLimit

So one has to go to extraordinary lengths that are completely non-obvious in the client code to actually pass-back the ammount allocated.  Here's a client in readImageFromFile:

        "allocate a contiguous block of memory for the Squeak heap" 
        memory := self cCode: 'sqAllocateMemory(minimumMemory, heapSize)'. 
        memory = nil ifTrue: [self insufficientMemoryAvailableError]. 

        memStart := self startOfMemory. 
        self setMemoryLimit: (memStart + heapSize) - 24. "decrease memoryLimit a tad for safety" 
        self setEndOfMemory: memStart + dataSize. 

and here's how one gets around the absence of a "how much was allocated" parameter, from platforms/Mac OS/vm/sqPlatformSpecifc.h

#undef sqAllocateMemory

usqInt      sqAllocateMemoryMac(sqInt minHeapSize, sqInt *desiredHeapSize);

#define sqAllocateMemory(x,y) sqAllocateMemoryMac(x,&y);

Ingenious (seriously; John this is a very clever working around of the restriction).  Totally non-obvious at the client site, but it still gets the job done without requiring the client to change.  So this costs too much to maintain (a.k.a. puts too much strain on my rodent brain).

We could go with an explicit out parameter, as in

        "allocate a contiguous block of memory for the Squeak heap" 
        memory := self cCode: 'sqAllocateMemory(minimumMemory, heapSize, &allocatedSize)'. 
        memory = nil ifTrue: [self insufficientMemoryAvailableError]. 

        memStart := self startOfMemory. 
        self setMemoryLimit: (memStart + allocatedSize) - 24. "decrease memoryLimit a tad for safety" 
        self setEndOfMemory: memStart + dataSize. 

but out parameters are not supported by Smalltalk and the VM is a Smalltalk program goddammit.

So I want to canvas opinions on the following simplified proposal, which is to drop the oldLimit parameter from sqGrowMemoryBy, discard sqShrinkMemoryBy (it is superfluous given sqGrowMemoryBy can take a signed argument) and use self sqGrowMemoryBy: 0 to determine how much memory has been allocated:

        "allocate a contiguous block of memory for the Squeak heap" 
        memory := self sqAllocateAtLeast: minimumMemory AtMostMemory: heapSize. 
        memory = nil ifTrue: [self insufficientMemoryAvailableError]. 
        heapSize := self sqGrowMemoryBy: 0. 
        memStart := self startOfMemory. 
        self setMemoryLimit: (memStart + heapSize) - 24. "decrease memoryLimit a tad for safety" 
        self setEndOfMemory: memStart + dataSize.

with the obvious default implementations in e.g. platforms/Cross/vm/sqMemory.c

static unsigned long memoryLimit = 0;

void *
sqAllocateAtLeastAtMostMemory(unsigned long committed, unsigned long reserved)
{
   void *mem;
    if ((mem = malloc(reserved)) ~= 0)
        memoryLimit = reserved;
    else if ((mem = malloc(committed)) ~= 0)
        memoryLimit = committed;
    return mem;
}

unsigned long
sqGrowMemoryBy(sqInt delta)
{
    return memoryLimit;
}

and everywhere in the VM which expects the old sqGrowMemoryBy to answer a pointer to the limit must instead add the result to the base of allocated memory; i.e.

        limit := self sqGrowMemory: memoryLimit By: delta. 
        limit = memoryLimit ifFalse: 
                 [self setMemoryLimit: limit - 24. "remove a tad for safety" 
                  self initializeMemoryFirstFree: freeBlock] 

is rewritten as

        limit := memoryLimit + (self sqGrowMemoryBy: delta). 
        limit = memoryLimit ifFalse: 
                 [self setMemoryLimit: limit - 24. "remove a tad for safety" 
                  self initializeMemoryFirstFree: freeBlock]
Opinions?

best
Eliot