| re: Is this portable/c-std at all?
On Sat, 24 Jan 2004, SenderX wrote:[color=blue]
>
> I am porting a library from msvc++ to gcc and POSIX and was wondering
> if the following was even close to being portable/c-std:[/color]
[and then later SenderX also wrote:][color=blue]
>- pMem->iRefs = 1;
>+ pMem->uiRefs = 1;
>
>- if ( ! ( --pMem ) )
>+ if ( ! ( --pMem->uiRefs ) )
>
> That's what I get for just typing that in real fast![/color]
Many computer systems have incorporated a wonderful application
known as "cut-and-paste." You might investigate it; it's a great
time-saver, and completely removes such silly errors as these.
It's especially good to cut-and-paste when dealing with comp.lang.c,
because otherwise we can't tell whether the silly errors were
present in the original code, or merely introduced by a careless
typist.
I'm afraid that I've completely re-formatted your code; I started
with just removing the gratuitous blank lines, then sensiblifying
the function definitions, and before I knew it I was chopping up
your text too much to pretend it was insignificant. I left the
quote > marks, though, to distinguish your code from my comments.
[color=blue]
> #define MEM_CAST unsigned char*
>
> #define MEM_TO_OBJ(m, o) ( (void*) (((MEM_CAST)m) + sizeof (o)) )
> #define OBJ_TO_MEM(m, o) ( (void*) (((MEM_CAST)m) - sizeof (o)) )[/color]
These macros are quite dubious even without looking closely; you
should never need to cast anything explicitly in well-written C code,
except in two or three highly exceptional cases.
Looking closely, we see that you've forgotten to parenthesize 'm'
in both cases. For full generality, it should read
[color=blue]
> #define MEM_TO_OBJ(m, o) ( (void*) ((MEM_CAST)(m) + sizeof (o)) )
> #define OBJ_TO_MEM(m, o) ( (void*) ((MEM_CAST)(m) - sizeof (o)) )[/color]
This will properly handle cases like 'MEM_TO_OBJ(p+1, a)'.
[color=blue]
> /* On memory released */
> typedef void (*ftMemHeaderOnReleased)(void *pUserState);
>
> /* Memory header */
> typedef struct SMemHeader_
> {
> unsigned int uiRefs;
> ftMemHeaderOnReleased clbkOnReleased;
> } SMemHeader, *SPMemHeader;
>
> /* Allocs memory */
> void *acMalloc(size_t stSz, ftMemHeaderOnReleased clbkOnReleased)
> {
> SPMemHeader pMem;
> stSz += sizeof *pMem;
> if ( ! (pMem = malloc(stSz)))
> return 0;
> pMem->uiRefs = 1;
> pMem->clbkOnReleased = clbkOnReleased;[/color]
We must assume here that 'clbkOnReleased' is properly declared
somewhere above as either
void clbkOnReleased(void *);
or
void (*clbkOnReleased)(void *);
Otherwise, you'll have a constraint violation here.
[color=blue]
> return MEM_TO_OBJ(pMem, *pMem);[/color]
This line is perfectly correct, but could more legibly be written
without the obfuscating macro and the dubious casts, as
return pMem+1;
[color=blue]
> }
>
> /* Releases memory */
> void acRelease ( void *pUserState )
> {
> SPMemHeader pMem = OBJ_TO_MEM( pUserState, *pMem );[/color]
Again, a simple
SPMemHeader pMem = pUserState;
pMem -= 1;
would have sufficed.
[color=blue]
> if ( ! ( --pMem->uiRefs ) )
> {
> if ( pMem->clbkOnReleased )
> pMem->clbkOnReleased( pUserState );
> free( pMem );
> }
> }[/color]
Note that you are ignoring the possibility that the user will pass
you a block with 'uiRefs' already zero (or negative), or that he'll
pass you a null pointer. That's probably intentional, though.
[color=blue]
> If this is not going to work portably, could it possibly be accomplished?[/color]
Looks portable enough to me. One caveat: You may *not* portably
assume that the pointer returned from 'acMalloc' is properly aligned
to store any type of object except the various flavors of 'char',
'int', and 'struct SMemHeader_'. Trying to write
double *p = acMalloc(sizeof *p, NULL);
will get you undefined behavior, even if it will *probably* work on
most platforms you'll encounter. There is no 100% correct C solution
to this dilemma; that's why the C standard library provides a 'malloc'
function with this "magic alignment" property, so you don't have to
write it.
-Arthur |