Re: PATCH: implemented Fiber APIs

[Dimitrie O. Paun]

On December 16, 2002 02:37 pm, Alberto Massari wrote:
> +void WINAPI SwitchToFiber(LPVOID lpFiber)
> +{
> +#ifdef HAVE_UCONTEXT_H
> - PHFIBER pFiber,pCurrentFiber;
> - pFiber=(PHFIBER)lpFiber;
> - pCurrentFiber=(PHFIBER)TlsGetValue(fiber_tls_index);
> - TRACE("Switching from %p to %p\n",pFiber,pCurrentFiber);
> - TlsSetValue(fiber_tls_index,pFiber);
> - swapcontext(&pCurrentFiber->context,&pFiber->context);
>
> +#else
> - FIXME("stub\n");
>
> +#endif
> +}

Can you (pretty) please do without the ifdefs? They can probably
be located in only one spot, if you need to test for HAVE_UCONTEXT_H.

PATCH: implemented Fiber APIs

[Alberto Massari]

At 02.02 17/12/2002 -0500, Dimitrie O. Paun wrote:
> On December 16, 2002 02:37 pm, Alberto Massari wrote:
> > +void WINAPI SwitchToFiber(LPVOID lpFiber)
> > +{
> > +#ifdef HAVE_UCONTEXT_H
> > - PHFIBER pFiber,pCurrentFiber;
> > - pFiber=(PHFIBER)lpFiber;
> > - pCurrentFiber=(PHFIBER)TlsGetValue(fiber_tls_index);
> > - TRACE("Switching from %p to %p\n",pFiber,pCurrentFiber);
> > - TlsSetValue(fiber_tls_index,pFiber);
> > - swapcontext(&pCurrentFiber->context,&pFiber->context);
> >
> > +#else
> > - FIXME("stub\n");
> >
> > +#endif
> > +}
>
> Can you (pretty) please do without the ifdefs? They can probably
> be located in only one spot, if you need to test for HAVE_UCONTEXT_H.

I added them when I realized that ucontext.h could not be present on some 
systems, as "configure" tests for its presence and adds that preprocessor 
directive.
The alternative layout would be

#ifdef HAVE_UCONTEXT_H

#include <ucontext.h>

LPVOID WINAPI ConvertThreadToFiber(...
{
   ....
}

LPVOID WINAPI CreateFiber(...
{
   ....
}
....

#else

LPVOID WINAPI ConvertThreadToFiber(...
{
    FIXME("stub\n");
}

LPVOID WINAPI CreateFiber(...
{
    FIXME("stub\n");
}
...

#endif

It's just a matter of deciding what is best from a maintenance point of 
view: grouping the working code or grouping the functions.
There are valid arguments in favour of both styles, and I have no strong 
feelings, so I will submit the new patch shortly.
In any case... what is the policy in this case?
a) there is an HOWTO on the subject, that I didn't read :-(
b) there is no previous agreement on this, and Alexandre decides
c) there is no previous agreement on this, and a poll is submitted to the 
mailing list
d) other options

Alberto

-------------------------------
Alberto Massari
eXcelon Corp.
http://www.StylusStudio.com

PATCH: implemented Fiber APIs

[Dimitrie O. Paun]

On December 17, 2002 04:59 am, Alberto Massari wrote:
> It's just a matter of deciding what is best from a maintenance point of
> view: grouping the working code or grouping the functions.

I would say grouping the working code is preferable (second patch you sent)
as the ugliness does not interfere with one's reading of the code.
> There are valid arguments in favour of both styles, and I have no strong
> feelings, so I will submit the new patch shortly.
> In any case... what is the policy in this case?

In general we don't go for either method, but rather we test for the
feature in port.h, and if absent we have our own implementation in
port.c. But it doesn't work well in this case.
> From my point of view, your second patch is better than the first.

Another way of doing it is stubbing the *context functions like so:

#ifndef HAVE_U_CONTEXT
typedef struct ucontext {
} ucontext_t;

int getcontext (ucontext_t *ucp)
{
    return -1;
}

int setcontext (const ucontext_t *ucp)
{
    return -1;
}

int swapcontext (ucontext_t *oucp, ucontext_t *ucp)
{
    return -1;
}

void makecontext(ucontext_t *ucp, void *func(), int argc, ...)
{
    return -1;
}
#endif

PATCH: implemented Fiber APIs

[Alexandre Julliard]

Alberto Massari  writes:
> It's just a matter of deciding what is best from a maintenance point
> of view: grouping the working code or grouping the functions.
> There are valid arguments in favour of both styles, and I have no
> strong feelings, so I will submit the new patch shortly.
> In any case... what is the policy in this case?

In general, the right approach is to implement an emulation for the
missing functions in port.c and don't have a single #ifdef inside the
code itself. In this particular case, the right fix is to not use
ucontext at all, but setjmp/longjmp instead. Actually I have a patch
that does this, I'll merge it in.