[ECOS] code to keep you on your toes..

[ECOS] code to keep you on your toes..

[Dave Airlie]

While trawling eCos code I found this nice one 

#define HAL_THREAD_SWITCH_CONTEXT(_fspptr_,_tspptr_)                    \
        hal_thread_switch_context((CYG_ADDRESS)_tspptr_,                \
                                  (CYG_ADDRESS)_fspptr_);

now that code is just plain evil... I'm assuming it is right as nothing
should work if it isn't, but having a function take args backwards to the
macro.. uggh...

I'd like to see that making it through a code review alive.

Dave.

-- 
      David Airlie, Software Engineer, Parthus Technologies plc.,
       Mary Rosse Centre, National Tech Park, Limerick, Ireland.
   t: +353-61-508116 / f: +353-61-508101 / David.Airlie@parthus.com

Re: [ECOS] code to keep you on your toes..

[Hugo 'NOx' Tyson]

"Dave Airlie" <airlied@parthus.com> writes:
> While trawling eCos code I found this nice one 
> 
> #define HAL_THREAD_SWITCH_CONTEXT(_fspptr_,_tspptr_)                    \
>         hal_thread_switch_context((CYG_ADDRESS)_tspptr_,                \
>                                   (CYG_ADDRESS)_fspptr_);
> 
> now that code is just plain evil... I'm assuming it is right as nothing
> should work if it isn't, but having a function take args backwards to the
> macro.. uggh...
> 
> I'd like to see that making it through a code review alive.

;-)  But...

There is a good reason for this, *given* the externally
defined standard HAL API for HAL_THREAD_SWITCH_CONTEXT(old,new)

The routine drops through to hal_thread_load_context( new-thread ), so you
want the new context in the first argument position.  Hence the old context
is placed in the 2nd argument position.

But the exported API (the macro) has (old,new) sematics which are thought
to be more friendly.  Or maybe this was arbitrarily chosen.  Don't
remember. ;-)  I think (old,new) reads more naturally.

My point is, this might argue for the external supported HAL API to be
reversed, but it's too late for that, since it's an external supported API.
But there is a good reason for the internal ABI to be the way it is, so we
won't change that.

	- Huge