On 6/11/24 6:05 PM, jor...@apache.org wrote:
> Author: jorton
> Date: Tue Jun 11 16:05:22 2024
> New Revision: 1918258
>
> URL: http://svn.apache.org/viewvc?rev=1918258&view=rev
> Log:
> * file_io/unix/pipe.c (file_pipe_create): Use pipe2(,O_NONBLOCK) by
> default unless APR_FULL_BLOCK was used; unconditionally set the
> blocking state later. Saves two syscalls per invocation for both
> APR_READ_BLOCK and APR_WRITE_BLOCK, no [intended] functional change.
I guess it is me being blind , but how do we save two syscalls for the
APR_READ_BLOCK and APR_WRITE_BLOCK case each? With the code before the patch
we started with a blocking pipe and needed to set one end of the pipe to non
blocking.
Now we start with a non blocking pipe and need to set the other end to blocking.
Where did I get lost and miss the point?
Regards
Rüdiger
>
> Github: closes #56
>
> Modified:
> apr/apr/trunk/file_io/unix/pipe.c
>
> Modified: apr/apr/trunk/file_io/unix/pipe.c
> URL:
> http://svn.apache.org/viewvc/apr/apr/trunk/file_io/unix/pipe.c?rev=1918258&r1=1918257&r2=1918258&view=diff
> ==============================================================================
> --- apr/apr/trunk/file_io/unix/pipe.c (original)
> +++ apr/apr/trunk/file_io/unix/pipe.c Tue Jun 11 16:05:22 2024
> @@ -186,7 +186,10 @@ static apr_status_t file_pipe_create(apr
> apr_interval_time_t fd_timeout;
>
> #ifdef HAVE_PIPE2
> - if (blocking == APR_FULL_NONBLOCK) {
> + /* If pipe2() is available, use O_NONBLOCK by default unless
> + * APR_FULL_BLOCK is requested, then set the individual pipe state
> + * as desired later - changing the state uses two syscalls. */
> + if (blocking != APR_FULL_BLOCK) {
> if (pipe2(filedes, O_NONBLOCK) == -1) {
> return errno;
> }
> @@ -239,21 +242,14 @@ static apr_status_t file_pipe_create(apr
> apr_pool_cleanup_register((*out)->pool, (void *)(*out),
> apr_unix_file_cleanup,
> apr_pool_cleanup_null);
>
> - switch (blocking) {
> - case APR_FULL_BLOCK:
> - break;
> - case APR_READ_BLOCK:
> - apr_file_pipe_timeout_set(*out, 0);
> - break;
> - case APR_WRITE_BLOCK:
> - apr_file_pipe_timeout_set(*in, 0);
> - break;
> - default:
> - /* These are noops for the pipe2() case */
> - apr_file_pipe_timeout_set(*out, 0);
> - apr_file_pipe_timeout_set(*in, 0);
> - break;
> - }
> + /* Set each pipe to the desired O_NONBLOCK state, which may be a
> + * noop depending on whether the pipe2() or pipe() case was
> + * used. (timeout of -1 == blocking) */
> + apr_file_pipe_timeout_set(*in, (blocking == APR_FULL_BLOCK
> + || blocking == APR_READ_BLOCK) ? -1 : 0);
> + apr_file_pipe_timeout_set(*out, (blocking == APR_FULL_BLOCK
> + || blocking == APR_WRITE_BLOCK) ? -1 :
> 0);
> +
> return APR_SUCCESS;
> }
>
>
>
>
