Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Is this portable/c-std at all?

Reply
Thread Tools

Is this portable/c-std at all?

 
 
SenderX
Guest
Posts: n/a
 
      01-24-2004
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:




#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 ) ) )




/* 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->iRefs = 1;

pMem->clbkOnReleased = clbkOnReleased;

return MEM_TO_OBJ( pMem, *pMem );
}


/* Releases memory */
void acRelease
(
void *pUserState
)

{
SPMemHeader pMem = OBJ_TO_MEM( pUserState, *pMem );

if ( ! ( --pMem ) )
{
if ( pMem->clbkOnReleased )
{
pMem->clbkOnReleased( pUserState );
}

free( pMem );
}
}


If this is not going to work portably, could it possibly be accomplished?


Thank you!




 
Reply With Quote
 
 
 
 
SenderX
Guest
Posts: n/a
 
      01-24-2004
> pMem->iRefs = 1;



I meant: pMem->uiRefs = 1;



> if ( ! ( --pMem ) )


DOH!

I meant: if ( ! ( --pMem->uiRefs ) )



That's what I get for just typing that in real fast!


 
Reply With Quote
 
 
 
 
Arthur J. O'Dwyer
Guest
Posts: n/a
 
      01-24-2004

On Sat, 24 Jan 2004, SenderX wrote:
>
> 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:


[and then later SenderX also wrote:]
>- pMem->iRefs = 1;
>+ pMem->uiRefs = 1;
>
>- if ( ! ( --pMem ) )
>+ if ( ! ( --pMem->uiRefs ) )
>
> That's what I get for just typing that in real fast!


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.


> #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)) )


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

> #define MEM_TO_OBJ(m, o) ( (void*) ((MEM_CAST)(m) + sizeof (o)) )
> #define OBJ_TO_MEM(m, o) ( (void*) ((MEM_CAST)(m) - sizeof (o)) )


This will properly handle cases like 'MEM_TO_OBJ(p+1, a)'.


> /* 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;


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.


> return MEM_TO_OBJ(pMem, *pMem);


This line is perfectly correct, but could more legibly be written
without the obfuscating macro and the dubious casts, as

return pMem+1;

> }
>
> /* Releases memory */
> void acRelease ( void *pUserState )
> {
> SPMemHeader pMem = OBJ_TO_MEM( pUserState, *pMem );


Again, a simple

SPMemHeader pMem = pUserState;
pMem -= 1;

would have sufficed.

> if ( ! ( --pMem->uiRefs ) )
> {
> if ( pMem->clbkOnReleased )
> pMem->clbkOnReleased( pUserState );
> free( pMem );
> }
> }


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.

> If this is not going to work portably, could it possibly be accomplished?


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
 
Reply With Quote
 
SenderX
Guest
Posts: n/a
 
      01-25-2004
> 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.


That would be due to an illegal thread reference.




> 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.


This allocation fuction is private, only library internals call it. The real
acMalloc trys to align to the cpu specific cache line. The lib knows what
structs have to be aligned to special boundaries, and takes special care to
ensure they are aligned. The system crashes real quick if it trys to use an
unaligned node in any lock-free algorithms, because they require cpu
specific atomic ops that might depend on proper alignment.




Thanks for your detailed response.


 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off




Advertisments