Re: [PATCH] My version of pipe-filter

[Bruno Haible]

Hi Paolo,

> here is a version of pipe-filter that works under Win32
> too (tested with Wine), has tests, and so on.
> 
> Testing under native Windows would be appreciated.
> 
> Bruno, what do you think?

I've added yours under the name 'pipe-filter-gi' and mine under the name
'pipe-filter-ii'. The 'g' stands for the "generator" and the 'i' for the
iterator argument type.

Your code and test suite prompted changes also for pipe-filter-ii:
  - argv argument is now 'const char **' instead of 'char **'.
  - Handling of write to closed pipe on Woe32: Here errno is EINVAL by
    default instead of EPIPE on Unix.
  - Handle the case that create_pipe_bidi returns -1.
  - Set SIGPIPE handler to SIG_IGN while writing to the subprocess.
  - Optim: Initialize readfds and writefds once, outside the loop.

On the side of pipe-filter-gi, I made the following changes:
* Behaviour:
  - When the child closes its standard input while the parent is
    still writing on it, treat it like an error, with errno = EPIPE.
    Normally this situation indicates that the parent and child
    don't agree on a protocol, or that the child exited prematurely.
* API:
  - Use 2 callback functions for the reader iterator, not just one.
    This allows to avoid memcpy in some use cases.
  - When there was an error and the child process returns, return
    the error code, not the child's exit status.
* Details:
  - Don't use assert. Use tests that are always compiled into the code.
  - In filter_terminate: Pass finish_reading = true instead of
    finish_reading = false when there had been a write error but no
    read error.
  - Use the same concept of return value in filter_write as in
    filter_close. filter_loop no longer leaves a result in errno.
  - Handle the case that create_pipe_bidi returns -1.
  - Speed up the test suite: fewer bash 'read' calls, but use longer
    strings to compensate.
* Details for the Unix implementation:
  - Set SIGPIPE handler to SIG_IGN while writing to the subprocess.
  - Don't use a 0 timeout for select(); that's a recipe for producing
    CPU-burning loops. When select() returns 0, abort.
  - Correct the logic in filter_loop: Use the same if() conditions,
    not 'if (!f->writer_errno)' in one place and 'if (count)' in another.
  - Also in filter_loop, when !HAVE_SELECT: A 'continue' statement
    was wrong.
  - Also in filter_loop: Terminate the loop when count becomes 0.
  - Also in filter_loop: When read() returns 0, don't terminate the
    loop. Instead, continue to write the given data.
* Details for the Woe32 implementation:
  - For synchronization between the two threads, use a CRITICAL_SECTION
    object instead of a mutex HANDLE. It's faster. Mutexes are needed
    only when synchronizing threads in different processes.
  - In reader_thread_func, don't ignore the errno from the read() call.
  - In filter_loop, use write() repeatedly, to protect against short
    writes. Also protect against count > INT_MAX.

Bruno