Questions about AllocMem()-, AllocVec()-alignment and Wipeout

[hceline]

I'm trying to modify Wipeout to do memory-tracking/leak-detection only.
While hunting down a bug I created by deleting too much code when removing the memory-wall code, I found some things that puzzled me.

The first is the alignment of the allocations.
One of the first things done by the function that preform the allocation is to pad the requested size to be dividable by MEM_BLOCKSIZE.
It then goes on to add the sizes of its own headers to this, before preforming the actual allocation.
Resulting in an actual allocation that is not always dividable by MEM_BLOCKSIZE, which seems kind of wrong to me.
Am I right in thinking that to be completely correct, the actual allocation should also be dividable by MEM_BLOCKSIZE?

The second thing that tripped me up was the storing of size in AllocVec() allocations.
It adds the size of the "size longword" to the variable "memSize" before the allocation is done.
And then it stores "memSize + sizeof(ULONG)" in the "size longword" at the end.
This can't be right?

And finally.
If I am right about that the actual allocation should be dividable by MEM_BLOCKSIZE, I assume this should be the total allocation including the "size longword" for AllocVec().
Is this right?

Code in question follows.


-Hagbard Celine

BOOL
PerformAllocation(
    ULONG *                    pc,
    struct PoolHeader *        poolHeader,
    ULONG                    memSize,
    ULONG                    attributes,
    UBYTE                    type,
    APTR *                    resultPtr)
{
    struct TrackHeader * th;
    ULONG allocationRemainder;
    ULONG postSize;
    LONG nameTagLen;
    APTR result = NULL;
    BOOL success = FALSE;

    nameTagLen = 0;

    /* Get the name of the current task, if this is necessary. */
    if(NameTag)
        nameTagLen = GetNameTagLen(*pc);

    /* If the allocation is not a multiple of the memory granularity
     * block size, increase the post memory wall by the remaining
     * few bytes of padding.
     */
    allocationRemainder = (memSize % MEM_BLOCKSIZE);
    if(allocationRemainder > 0)
        allocationRemainder = MEM_BLOCKSIZE - allocationRemainder;

    postSize = allocationRemainder;

    switch(type)
    {
        case ALLOCATIONTYPE_AllocMem:

            th = (*OldAllocMem)(nameTagLen + TrackHeaderSize + memSize + postSize,attributes & (~MEMF_CLEAR),SysBase);
            if(th != NULL)
            {
                /* Store the name tag data in front of the header, then
                 * adjust the header address.
                 */
                if(nameTagLen > 0)
                {
                    FillNameTag(th,nameTagLen);
                    th = (struct TrackHeader *)(((ULONG)th) + nameTagLen);
                }

                AddAllocation(th);
            }

            break;

        case ALLOCATIONTYPE_AllocVec:

            /* This will later contain the length long word. */
            memSize += sizeof(ULONG);

            th = (*OldAllocMem)(nameTagLen + TrackHeaderSize + memSize + postSize,attributes & (~MEMF_CLEAR),SysBase);
            if(th != NULL)
            {
                /* Store the name tag data in front of the header, then
                 * adjust the header address.
                 */
                if(nameTagLen > 0)
                {
                    FillNameTag(th,nameTagLen);
                    th = (struct TrackHeader *)(((ULONG)th) + nameTagLen);
                }

                AddAllocation(th);
            }

            break;

        case ALLOCATIONTYPE_AllocPooled:

            th = (*OldAllocPooled)(poolHeader->ph_PoolHeader,nameTagLen + TrackHeaderSize + memSize + postSize,SysBase);
            if(th != NULL)
            {
                /* Store the name tag data in front of the header, then
                 * adjust the header address.
                 */
                if(nameTagLen > 0)
                {
                    FillNameTag(th,nameTagLen);
                    th = (struct TrackHeader *)(((ULONG)th) + nameTagLen);
                }

                AddPuddle(poolHeader,th);
            }

            break;

        default:

            th = NULL;
            break;
    }

    if(th != NULL)
    {
        STATIC ULONG Sequence;

        UBYTE * mem;

        /* Fill in the regular header data. */
        th->th_Magic        = BASEBALL;
        th->th_PointBack    = th;
        th->th_PC            = *pc;
        th->th_Owner        = FindTask(NULL);
        th->th_OwnerType    = GetTaskType(NULL);
        th->th_NameTagLen    = nameTagLen;

        GetSysTime(&th->th_Time);
        th->th_Sequence = Sequence++;

        th->th_Size            = memSize;
        th->th_PoolHeader    = poolHeader;
        th->th_Type            = type;
        th->th_PostSize        = postSize;
        th->th_Marked        = FALSE;

        if (StackCacheSize)
        {
            ULONG * stackmem;
            int i,j;

            stackmem = &pc[1];
            j = 0;

            th->th_StackCache[j].sc_sourceName[0] = 0;

            for(i = 0 ; (void *)&stackmem[i] <= th->th_Owner->tc_SPUpper && j < StackCacheSize ; i++)
            {
                if ((stackmem[i] > 0x4000) && (TypeOfMem((APTR)stackmem[i])))
                {
                    if (FindAddressEx(stackmem[i],
                                      th->th_StackCache[j].sc_sourceName,
                                      &th->th_StackCache[j].sc_lineNum,
                                      &th->th_StackCache[j].sc_lineOffset))
                    {
                        th->th_StackCache[j].sc_stack = stackmem[i];
                        ++j;
                        if ( j < StackCacheSize )
                            th->th_StackCache[j].sc_sourceName[0] = 0;

                    }

                }
            }
            while (j < (StackCacheSize - 1))
            {
                ++j;
                th->th_StackCache[j].sc_sourceName[0] = 0;
            }
        }

        /* Calculate the checksum. */
        FixTrackHeaderChecksum(th);

        /* Fill in the preceding memory wall. */
        mem = (UBYTE *)(th + 1);
        mem += StackHeaderSize;

        /* Zero the memory allocation body if
         * requested.
         */
        if(FLAG_IS_SET(attributes,MEMF_CLEAR))
            memset(mem,0,memSize);

        /* AllocVec()'ed allocations are special in that
         * the size of the allocation precedes the header.
         */
        if(type == ALLOCATIONTYPE_AllocVec)
        {
            /* Size of the allocation must include the
             * size long word.
             */
            (*(ULONG *)mem) = memSize + sizeof(ULONG);

            result = (APTR)(mem + sizeof(ULONG));
        }
        else
        {
            result = (APTR)mem;
        }

        success = TRUE;
    }

    (*resultPtr) = result;

    return(success);
}

[hceline]

After mulling it over, I realized I could easily check this by doing a few "result=AllocVec()" and checking "result[-1]".
And the correct answers seem to be:

1. Yes, the final allocation should be aligned for AllocVec() to match original behavior. AllocMem() does not do alignment of the end of allocation by design (obviously ).

2. One of the duplicate "sizeof(ULONG)" should be replaced with "the size of alignment padding".

3. Yes.

-Hagbard Celine