Re: [bug #33138] .PARLLELSYNC enhancement with patch

[Eli Zaretskii]

> Date: Thu, 25 Apr 2013 05:14:41 +0200
> Cc: address@hidden, address@hidden
> From: Frank Heckenbach <address@hidden>
> 
> My suggestion was under the assumption that we use different
> declarations because of different types anyway -- which I still
> recommend, though Paul might have to decide this one.

Yes, eventually it's Paul's call.

> If I understand it correctly, roughly we'd have now:
> 
> intptr_t sync_handle;
> 
> [...]
> #ifdef POSIX
> sync_handle = (intptr_t) fileno (stdout);
> #else
> sync_handle = (intptr_t) get_mutex_handle (...);
> #endif
> [...]
> 
> #ifdef POSIX
> fcntl ((int) sync_handle, ...);
> #else
> lock_mutex ((mutex *) sync_handle);
> #endif
> 
> I.e., everything about it is separate, apart from the declaration,
> and the latter uses a type that isn't quite right for either one.

You underestimate me ;-)

What I have is actually this:

sync_handle_t sync_handle = -1;
[...]
#if POSIX
  sync_handle = fileno (stdout);
#elif WINDOWS
  sync_handle = create_mutex ();
#endif
[...]
  if (fcntl (sync_handle, F_SETLKW, &fl) != -1)
    return &fl;

and I wrote 'fcntl' emulation for Windows that uses a mutex.

IOW, I didn't add even a single ifdef (the one you see above was
already there, and it can be removed if we try hard enough).

That said, I'm not wedded to the above approach, and if people like a
completely disjoint code for Windows, that would be a trivial change.
I just wanted to comply with what Paul said, viz.:

> > Also, where is the best place to put the emulated Posix functions?
> > Some new file in w32/compat/? 
> 
> I'd like to see it there.  I'm thinking I want to move the new stuff out
> of job.c even for POSIX systems.  The ifdefs are really getting to me.

So I now have w32/compat/posixfcn.c with the emulation of fcntl (also
used for CLOSE_ON_EXEC), and a few support functions it needs.

> Though I generally like reducing the number of ifdefs, in this case
> it seems more consistent to split the declaration, so it's clear
> that these are really two different things. As a side benefit, if in
> the future someone tried to use it in common code, they would get
> compile-time errors on the other platform, rather than silently
> producing wrong code (like applying some fd operation to a mutex
> handle).

If we really want to separate Posix code from non-Posix one, I would
suggest to have identically named functions, such as
start_job_command, construct_command_argv, etc., in 2 separate files.
These are very large and complex functions, and it is very hard to
follow their logic when large portions of them are more-or-less copied
with non-trivial changes, and you have something like

#ifdef MSDOS
 /* lots of MSDOS stuff */
#else
#ifdef WINDOWS
 /* lots of similar-but-different Windows  stuff */
#else
#ifdef AMIGA
 /* Amiga stuff */
#else
#ifdef _EMX_
 /* EMX stuff */
#else
 /* Posix stuff */

Just paging through the irrelevant portions without losing track of
the logic is non-trivial.  You need a very good editor with folding
capabilities to not become distracted.