Re: [evolution-patches] e-d-s: pthread_t opaqueness patch

[Tor Lillqvist <tml novell com>]

On to, 2005-08-18 at 15:56 +0800, Not Zed wrote:
(about e_pthread_id())
> Umm, this is really broken.  It's a threading function, and it isn't
> thread-safe or reentrant.

(Blush) Yes, this was just a quick idea, and I wasn't really satisfied
with it... Hmm, returning a freshly allocated string would mean it would
have to be freed after each use, that means more code clutter to stuff
mostly used just by debugging anyway. Sigh. I'll change it to return
just a long, which on sane platforms would be the pthread_t itself. On
Win32 I can use the part of the internals of pthread_t that is unique.
On other random platforms where pthread_t is a struct, a hash of the
contents.

> It should use a feature test not a platform test too.

OK, I'll write the required configure.in snippet to find out whether a
pthread_t is something that can be printed with %lu. Hmm, I guess just
an AC_TRY_COMPILE() will do.

> What has this got to do with pthread_id?
> -                       sleep(1);
> +                       g_usleep(1000000);

Nothing. As I said I am trying to split up my total current diff into
logically related more easily digestable pieces, but when one source
file has several unrelated changes, some not related to the subject of
that particular patch message might slip in. 

(Microsoft's C library doesn't implement POSIX sleep(), sigh. If the
g_sleep() call are really irritating (I must admit it looks a bit silly
with all those zeros), I could handle this with a macro instead, of
course.) 

> I must say, the e-msgport changes are a tad bigger than 'only a few
> lines'.  Thats why i said to use a new file to implement it. 

Well, there is one major added code block (the definitions of the socket
API wrapper macros and the e_pipe() function), the rest of the
e-msgport.c diff is just one- or two-line changes. All in all about 40
*changed* lines. Not really enough to justify a new copy of a 400-line
source file, where most of the contents would be identical, IMHO.  

> i'm going to hard-insist that you get the style right.  e.g.
> no space before ( for function calls.

OK, will be more careful.

> Also, don't add a has_owner to the object-bag stuff.  It is only used
> for assert tests where equality & memset to 0 will do too.

But can one be sure that in all pthread implementations a zero-filled
pthread_t is invalid? It isn't hard to imagine that some implementation
might use a simple increasing integer as pthread_t, starting from zero?

--tml