You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by bj...@apache.org on 2010/04/03 17:43:00 UTC
svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Author: bjh
Date: Sat Apr 3 15:43:00 2010
New Revision: 930533
URL: http://svn.apache.org/viewvc?rev=930533&view=rev
Log:
Add missing APR_DECLARE for apr_wait_for_io_or_timeout() declaration.
Modified:
apr/apr/trunk/include/apr_support.h
Modified: apr/apr/trunk/include/apr_support.h
URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_support.h?rev=930533&r1=930532&r2=930533&view=diff
==============================================================================
--- apr/apr/trunk/include/apr_support.h (original)
+++ apr/apr/trunk/include/apr_support.h Sat Apr 3 15:43:00 2010
@@ -45,8 +45,9 @@ extern "C" {
* otherwise wait for data to be able to be written.
* @return APR_TIMEUP if we run out of time.
*/
-apr_status_t apr_wait_for_io_or_timeout(apr_file_t *f, apr_socket_t *s,
- int for_read);
+APR_DECLARE(apr_status_t) apr_wait_for_io_or_timeout(apr_file_t *f,
+ apr_socket_t *s,
+ int for_read);
/** @} */
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Mladen Turk <mt...@apache.org>.
On 04/03/2010 05:43 PM, bjh@apache.org wrote:
> Author: bjh
> Date: Sat Apr 3 15:43:00 2010
> New Revision: 930533
>
> URL: http://svn.apache.org/viewvc?rev=930533&view=rev
> Log:
> Add missing APR_DECLARE for apr_wait_for_io_or_timeout() declaration.
>
I don't think this was missing.
apr_wait_for_io_or_timeout was not a public API which you
have just made.
See what apr_support.h says:
@defgroup apr_support Internal APR support functions
Regards
--
^TM
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Mladen Turk <mt...@apache.org>.
On 04/07/2010 01:31 PM, William A. Rowe Jr. wrote:
> By rights, we should be moving apr_support.h out of include/, that was a first
> mistake. I have the same understanding as you, Mladen, but the authors of the
> httpd mod_reqtimeout seem to be reaching their fingers into this header.
>
If that's the case then the API could be declared as public,
but it'll have to be defined in another header file.
Regards
--
^TM
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 4/7/2010 12:14 PM, Jeff Trawick wrote:
> On Wed, Apr 7, 2010 at 12:59 PM, Ruediger Pluem <rp...@apache.org> wrote:
>> On 07.04.2010 13:31, William A. Rowe Jr. wrote:
>>> By rights, we should be moving apr_support.h out of include/, that was a first
>>> mistake. I have the same understanding as you, Mladen, but the authors of the
>>> httpd mod_reqtimeout seem to be reaching their fingers into this header.
>>
>> mod_proxy_fdpass does as well. So maybe we should make it public.
>
> For an external API that satisfies this use case, I'd rather see
> something like separate apr_file_wait()/apr_socket_wait() instead of
> the current interface. (They probably need enums for the I/O type
> too, instead of the int flag.)
>
> This can use wait-for-io-or-timeout internally if appropriate.
+1
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Jeff Trawick <tr...@gmail.com>.
On Mon, Apr 12, 2010 at 9:57 AM, Brian Havard <br...@gmail.com> wrote:
> Jeff Trawick wrote:
>
> On Thu, Apr 8, 2010 at 6:37 AM, Brian Havard <br...@gmail.com> wrote:
>
>
> Brian Havard wrote:
>
>
> Jeff Trawick wrote:
>
>
> [...]
>
>
> For an external API that satisfies this use case, I'd rather see
> something like separate apr_file_wait()/apr_socket_wait() instead of
> the current interface. (They probably need enums for the I/O type
> too, instead of the int flag.)
>
> This can use wait-for-io-or-timeout internally if appropriate.
>
>
>
> Sounds good. I can start hacking something together if there's agreement.
>
>
>
> Ok, here's what I've come up with.
>
> I haven't done a Win32 implementation because I'm not sure how to wait
> on a file/pipe. I could probably take a stab at doing apr_socket_wait()
> if I get around to digging out a windows compiler but probably best left
> to someone more familiar with apr on windows.
>
> Comments?
>
>
> looks reasonable here
>
>
> After thinking about this a bit more, I'd rather not have apr_file_wait() as
> waiting on regular files isn't portable or even particularly useful. I think
> it would be better to call it apr_file_pipe_wait() to make it clear that
> only pipe type apr_file_t handles are supported.
makes sense (we already have apr_file_pipe_FOO for several functions
that make sense only for pipe-as-apr_file_t)...
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Brian Havard <br...@gmail.com>.
Jeff Trawick wrote:
> On Thu, Apr 8, 2010 at 6:37 AM, Brian Havard <br...@gmail.com> wrote:
>
>> Brian Havard wrote:
>>
>>> Jeff Trawick wrote:
>>>
>> [...]
>>
>>>> For an external API that satisfies this use case, I'd rather see
>>>> something like separate apr_file_wait()/apr_socket_wait() instead of
>>>> the current interface. (They probably need enums for the I/O type
>>>> too, instead of the int flag.)
>>>>
>>>> This can use wait-for-io-or-timeout internally if appropriate.
>>>>
>>>>
>>> Sounds good. I can start hacking something together if there's agreement.
>>>
>>>
>> Ok, here's what I've come up with.
>>
>> I haven't done a Win32 implementation because I'm not sure how to wait
>> on a file/pipe. I could probably take a stab at doing apr_socket_wait()
>> if I get around to digging out a windows compiler but probably best left
>> to someone more familiar with apr on windows.
>>
>> Comments?
>>
>
> looks reasonable here
>
After thinking about this a bit more, I'd rather not have
apr_file_wait() as waiting on regular files isn't portable or even
particularly useful. I think it would be better to call it
apr_file_pipe_wait() to make it clear that only pipe type apr_file_t
handles are supported.
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, Apr 8, 2010 at 6:37 AM, Brian Havard <br...@gmail.com> wrote:
> Brian Havard wrote:
>> Jeff Trawick wrote:
> [...]
>>> For an external API that satisfies this use case, I'd rather see
>>> something like separate apr_file_wait()/apr_socket_wait() instead of
>>> the current interface. (They probably need enums for the I/O type
>>> too, instead of the int flag.)
>>>
>>> This can use wait-for-io-or-timeout internally if appropriate.
>>>
>>
>> Sounds good. I can start hacking something together if there's agreement.
>>
>
> Ok, here's what I've come up with.
>
> I haven't done a Win32 implementation because I'm not sure how to wait
> on a file/pipe. I could probably take a stab at doing apr_socket_wait()
> if I get around to digging out a windows compiler but probably best left
> to someone more familiar with apr on windows.
>
> Comments?
looks reasonable here
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Brian Havard <br...@gmail.com>.
Brian Havard wrote:
> Jeff Trawick wrote:
[...]
>> For an external API that satisfies this use case, I'd rather see
>> something like separate apr_file_wait()/apr_socket_wait() instead of
>> the current interface. (They probably need enums for the I/O type
>> too, instead of the int flag.)
>>
>> This can use wait-for-io-or-timeout internally if appropriate.
>>
>
> Sounds good. I can start hacking something together if there's agreement.
>
Ok, here's what I've come up with.
I haven't done a Win32 implementation because I'm not sure how to wait
on a file/pipe. I could probably take a stab at doing apr_socket_wait()
if I get around to digging out a windows compiler but probably best left
to someone more familiar with apr on windows.
Comments?
Index: test/testpipe.c
===================================================================
--- test/testpipe.c (revision 931631)
+++ test/testpipe.c (working copy)
@@ -186,6 +186,35 @@
ABTS_ASSERT(tc, "child terminated normally", why == APR_PROC_EXIT);
}
+static void wait_pipe(abts_case *tc, void *data)
+{
+ apr_status_t rv;
+ apr_interval_time_t delay = 200000;
+ apr_time_t start_time;
+ apr_time_t end_time;
+ apr_size_t nbytes;
+
+ rv = apr_file_pipe_create(&readp, &writep, p);
+ APR_ASSERT_SUCCESS(tc, "Couldn't create pipe", rv);
+
+ rv = apr_file_pipe_timeout_set(readp, delay);
+ APR_ASSERT_SUCCESS(tc, "Couldn't set pipe timeout", rv);
+
+ start_time = apr_time_now();
+ rv = apr_file_wait(readp, APR_WAIT_READ);
+ ABTS_INT_EQUAL(tc, 1, APR_STATUS_IS_TIMEUP(rv));
+
+ end_time = apr_time_now();
+ ABTS_ASSERT(tc, "apr_file_wait() waited for the time out", end_time
- start_time >= delay);
+
+ nbytes = 4;
+ rv = apr_file_write(writep, "data", &nbytes);
+ APR_ASSERT_SUCCESS(tc, "Couldn't write to pipe", rv);
+
+ rv = apr_file_wait(readp, APR_WAIT_READ);
+ APR_ASSERT_SUCCESS(tc, "Wait for pipe failed", rv);
+}
+
abts_suite *testpipe(abts_suite *suite)
{
suite = ADD_SUITE(suite)
@@ -199,6 +228,7 @@
abts_run_test(suite, read_write_notimeout, NULL);
abts_run_test(suite, test_pipe_writefull, NULL);
abts_run_test(suite, close_pipe, NULL);
+ abts_run_test(suite, wait_pipe, NULL);
return suite;
}
Index: test/testsock.c
===================================================================
--- test/testsock.c (revision 931631)
+++ test/testsock.c (working copy)
@@ -403,6 +403,64 @@
apr_socket_close(ld);
}
+static void test_wait(abts_case *tc, void *data)
+{
+ apr_status_t rv;
+ apr_socket_t *server;
+ apr_socket_t *server_connection;
+ apr_sockaddr_t *server_addr;
+ apr_socket_t *client;
+ apr_interval_time_t delay = 200000;
+ apr_time_t start_time;
+ apr_time_t end_time;
+ apr_size_t nbytes;
+ int connected = FALSE;
+
+ server = setup_socket(tc);
+ rv = apr_sockaddr_info_get(&server_addr, socket_name, socket_type,
8021, 0, p);
+ APR_ASSERT_SUCCESS(tc, "setting up sockaddr", rv);
+
+ rv = apr_socket_create(&client, server_addr->family, SOCK_STREAM,
0, p);
+ APR_ASSERT_SUCCESS(tc, "creating client socket", rv);
+
+ rv = apr_socket_timeout_set(client, 0);
+ APR_ASSERT_SUCCESS(tc, "setting client socket timeout", rv);
+
+ rv = apr_socket_connect(client, server_addr);
+
+ if (rv == APR_SUCCESS) {
+ connected = TRUE;
+ }
+ else {
+ ABTS_ASSERT(tc, "connecting client to server",
APR_STATUS_IS_EINPROGRESS(rv));
+ }
+
+ rv = apr_socket_accept(&server_connection, server, p);
+ APR_ASSERT_SUCCESS(tc, "accepting client connection", rv);
+
+ if (!connected) {
+ rv = apr_socket_connect(client, server_addr);
+ APR_ASSERT_SUCCESS(tc, "connecting client to server", rv);
+ }
+
+ rv = apr_socket_timeout_set(client, delay);
+ APR_ASSERT_SUCCESS(tc, "setting client socket timeout", rv);
+
+ start_time = apr_time_now();
+ rv = apr_socket_wait(client, APR_WAIT_READ);
+ ABTS_INT_EQUAL(tc, 1, APR_STATUS_IS_TIMEUP(rv));
+
+ end_time = apr_time_now();
+ ABTS_ASSERT(tc, "apr_socket_wait() waited for the time out",
end_time - start_time >= delay);
+
+ nbytes = 4;
+ rv = apr_socket_send(server_connection, "data", &nbytes);
+ APR_ASSERT_SUCCESS(tc, "Couldn't write to client", rv);
+
+ rv = apr_socket_wait(client, APR_WAIT_READ);
+ APR_ASSERT_SUCCESS(tc, "Wait for socket failed", rv);
+}
+
abts_suite *testsock(abts_suite *suite)
{
suite = ADD_SUITE(suite)
@@ -416,6 +474,7 @@
abts_run_test(suite, test_timeout, NULL);
abts_run_test(suite, test_print_addr, NULL);
abts_run_test(suite, test_get_addr, NULL);
+ abts_run_test(suite, test_wait, NULL);
#if APR_HAVE_SOCKADDR_UN
socket_name = UNIX_SOCKET_NAME;
socket_type = APR_UNIX;
@@ -423,6 +482,7 @@
abts_run_test(suite, test_send, NULL);
abts_run_test(suite, test_recv, NULL);
abts_run_test(suite, test_timeout, NULL);
+ abts_run_test(suite, test_wait, NULL);
#endif
return suite;
}
Index: network_io/os2/sendrecv.c
===================================================================
--- network_io/os2/sendrecv.c (revision 931631)
+++ network_io/os2/sendrecv.c (working copy)
@@ -153,3 +153,21 @@
*len = rv;
return APR_SUCCESS;
}
+
+
+
+APR_DECLARE(apr_status_t) apr_socket_wait(apr_socket_t *sock,
apr_wait_type_t direction)
+{
+ int pollsocket = sock->socketdes;
+ int wait_rc = select(&pollsocket, direction == APR_WAIT_READ,
+ direction == APR_WAIT_WRITE, 0, sock->timeout
/ 1000);
+
+ if (wait_rc == 0) {
+ return APR_TIMEUP;
+ }
+ else if (wait_rc < 0) {
+ return APR_FROM_OS_ERROR(sock_errno());
+ }
+
+ return APR_SUCCESS;
+}
Index: network_io/unix/sendrecv.c
===================================================================
--- network_io/unix/sendrecv.c (revision 931631)
+++ network_io/unix/sendrecv.c (working copy)
@@ -235,6 +235,11 @@
#endif
}
+apr_status_t apr_socket_wait(apr_socket_t *sock, apr_wait_type_t direction)
+{
+ return apr_wait_for_io_or_timeout(NULL, sock, direction ==
APR_WAIT_READ);
+}
+
#if APR_HAS_SENDFILE
/* TODO: Verify that all platforms handle the fd the same way,
Index: include/apr_support.h
===================================================================
--- include/apr_support.h (revision 931631)
+++ include/apr_support.h (working copy)
@@ -1,57 +0,0 @@
-/* Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements. See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef APR_SUPPORT_H
-#define APR_SUPPORT_H
-
-/**
- * @file apr_support.h
- * @brief APR Support functions
- */
-
-#include "apr.h"
-#include "apr_network_io.h"
-#include "apr_file_io.h"
-
-#ifdef __cplusplus
-extern "C" {
-#endif /* __cplusplus */
-
-/**
- * @defgroup apr_support Internal APR support functions
- * @ingroup APR
- * @{
- */
-
-/**
- * Wait for IO to occur or timeout.
- *
- * @param f The file to wait on.
- * @param s The socket to wait on if @a f is @c NULL.
- * @param for_read If non-zero wait for data to be available to read,
- * otherwise wait for data to be able to be written.
- * @return APR_TIMEUP if we run out of time.
- */
-apr_status_t apr_wait_for_io_or_timeout(apr_file_t *f, apr_socket_t *s,
- int for_read);
-
-/** @} */
-
-#ifdef __cplusplus
-}
-#endif
-
-#endif /* ! APR_SUPPORT_H */
Index: include/apr_general.h
===================================================================
--- include/apr_general.h (revision 931631)
+++ include/apr_general.h (working copy)
@@ -67,6 +67,9 @@
/** signal numbers typedef */
typedef int apr_signum_t;
+/* Type of I/O to wait for */
+typedef enum { APR_WAIT_READ, APR_WAIT_WRITE } apr_wait_type_t;
+
/**
* Finding offsets of elements within structures.
* Taken from the X code... they've sweated portability of this stuff
Index: include/apr_file_io.h
===================================================================
--- include/apr_file_io.h (revision 931631)
+++ include/apr_file_io.h (working copy)
@@ -569,6 +569,16 @@
APR_DECLARE(apr_status_t) apr_file_puts(const char *str, apr_file_t
*thefile);
/**
+ * Wait for a file or pipe to be ready for input or output
+ * @param thefile the file to wait on
+ * @param direction whether to wait for reading or writing to be ready
+ * @remark Will time out if thefile has a time out set for it
+ * @remark direction can be either APR_WAIT_READ or APR_WAIT_WRITE
+ */
+APR_DECLARE(apr_status_t) apr_file_wait(apr_file_t *thefile,
+ apr_wait_type_t direction);
+
+/**
* Flush the file's buffer.
* @param thefile The file descriptor to flush
*/
Index: include/apr_network_io.h
===================================================================
--- include/apr_network_io.h (revision 931631)
+++ include/apr_network_io.h (working copy)
@@ -621,6 +621,16 @@
char *buf, apr_size_t *len);
/**
+ * Wait for a socket to be ready for input or output
+ * @param sock the socket to wait on
+ * @param direction whether to wait for reading or writing to be ready
+ * @remark Will time out if socket has a time out set for it
+ * @remark direction can be either APR_WAIT_READ or APR_WAIT_WRITE
+ */
+APR_DECLARE(apr_status_t) apr_socket_wait(apr_socket_t *sock,
+ apr_wait_type_t direction);
+
+/**
* Setup socket options for the specified socket
* @param sock The socket to set up.
* @param opt The option we would like to configure. One of:
Index: file_io/os2/readwrite.c
===================================================================
--- file_io/os2/readwrite.c (revision 931631)
+++ file_io/os2/readwrite.c (working copy)
@@ -409,6 +409,31 @@
+APR_DECLARE(apr_status_t) apr_file_wait(apr_file_t *f, apr_wait_type_t
direction)
+{
+ int rc;
+
+ if (!f->pipe) {
+ /* No support for waiting on a regular file */
+ return APR_ENOTIMPL;
+ }
+
+ if (((f->flags & APR_FOPEN_READ) > 0) != (direction ==
APR_WAIT_READ)) {
+ /* Attempt to wait for read from the write end of the pipe or
vica versa */
+ return APR_EINVAL;
+ }
+
+ rc = DosWaitEventSem(f->pipeSem, f->timeout >= 0 ? f->timeout /
1000 : SEM_INDEFINITE_WAIT);
+
+ if (rc == ERROR_TIMEOUT) {
+ return APR_TIMEUP;
+ }
+
+ return APR_FROM_OS_ERROR(rc);
+}
+
+
+
APR_DECLARE(apr_status_t) apr_file_rotating_check(apr_file_t *thefile)
{
return APR_ENOTIMPL;
Index: file_io/os2/pipe.c
===================================================================
--- file_io/os2/pipe.c (revision 931631)
+++ file_io/os2/pipe.c (working copy)
@@ -81,7 +81,7 @@
(*in)->fname = apr_pstrdup(pool, pipename);
(*in)->isopen = TRUE;
(*in)->buffered = FALSE;
- (*in)->flags = 0;
+ (*in)->flags = APR_FOPEN_READ;
(*in)->pipe = 1;
(*in)->timeout = -1;
(*in)->blocking = BLK_ON;
@@ -113,7 +113,7 @@
(*out)->fname = apr_pstrdup(pool, pipename);
(*out)->isopen = TRUE;
(*out)->buffered = FALSE;
- (*out)->flags = 0;
+ (*out)->flags = APR_FOPEN_WRITE;
(*out)->pipe = 1;
(*out)->timeout = -1;
(*out)->blocking = BLK_ON;
Index: file_io/unix/readwrite.c
===================================================================
--- file_io/unix/readwrite.c (revision 931631)
+++ file_io/unix/readwrite.c (working copy)
@@ -562,3 +562,10 @@
}
return rv;
}
+
+
+
+APR_DECLARE(apr_status_t) apr_file_wait(apr_file_t *thefile,
apr_wait_type_t direction)
+{
+ return apr_wait_for_io_or_timeout(thefile, NULL, direction ==
APR_WAIT_READ);
+}
Index: support/os2/waitio.c
===================================================================
--- support/os2/waitio.c (revision 931631)
+++ support/os2/waitio.c (working copy)
@@ -19,51 +19,14 @@
#include "apr_errno.h"
#include "apr_support.h"
-static apr_status_t wait_for_file(apr_file_t *f, int for_read)
-{
- int rc;
-
- if (!f->pipe) {
- /* No support for waiting on a regular file */
- return APR_ENOTIMPL;
- }
-
- rc = DosWaitEventSem(f->pipeSem, f->timeout >= 0 ? f->timeout /
1000 : SEM_INDEFINITE_WAIT);
-
- if (rc == ERROR_TIMEOUT) {
- return APR_TIMEUP;
- }
-
- return APR_FROM_OS_ERROR(rc);
-}
-
-
-
-static apr_status_t wait_for_socket(apr_socket_t *s, int for_read)
-{
- int pollsocket = s->socketdes;
- int wait_rc = select(&pollsocket, for_read != 0, for_read == 0, 0,
s->timeout / 1000);
-
- if (wait_rc == 0) {
- return APR_TIMEUP;
- }
- else if (wait_rc < 0) {
- return APR_FROM_OS_ERROR(sock_errno());
- }
-
- return APR_SUCCESS;
-}
-
-
-
apr_status_t apr_wait_for_io_or_timeout(apr_file_t *f, apr_socket_t *s,
int for_read)
{
if (f) {
- return wait_for_file(f, for_read);
+ return apr_file_wait(f, for_read ? APR_WAIT_READ : APR_WAIT_WRITE);
}
else if (s) {
- return wait_for_socket(s, for_read);
+ return apr_socket_wait(s, for_read ? APR_WAIT_READ :
APR_WAIT_WRITE);
}
/* Didn't specify a file or socket */
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Brian Havard <br...@gmail.com>.
Jeff Trawick wrote:
> On Wed, Apr 7, 2010 at 12:59 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
>> On 07.04.2010 13:31, William A. Rowe Jr. wrote:
>>
>>> By rights, we should be moving apr_support.h out of include/, that was a first
>>> mistake. I have the same understanding as you, Mladen, but the authors of the
>>> httpd mod_reqtimeout seem to be reaching their fingers into this header.
>>>
>> mod_proxy_fdpass does as well. So maybe we should make it public.
>>
>
> For an external API that satisfies this use case, I'd rather see
> something like separate apr_file_wait()/apr_socket_wait() instead of
> the current interface. (They probably need enums for the I/O type
> too, instead of the int flag.)
>
> This can use wait-for-io-or-timeout internally if appropriate.
>
Sounds good. I can start hacking something together if there's agreement.
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Apr 7, 2010 at 12:59 PM, Ruediger Pluem <rp...@apache.org> wrote:
> On 07.04.2010 13:31, William A. Rowe Jr. wrote:
>> By rights, we should be moving apr_support.h out of include/, that was a first
>> mistake. I have the same understanding as you, Mladen, but the authors of the
>> httpd mod_reqtimeout seem to be reaching their fingers into this header.
>
> mod_proxy_fdpass does as well. So maybe we should make it public.
For an external API that satisfies this use case, I'd rather see
something like separate apr_file_wait()/apr_socket_wait() instead of
the current interface. (They probably need enums for the I/O type
too, instead of the int flag.)
This can use wait-for-io-or-timeout internally if appropriate.
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 07.04.2010 13:31, William A. Rowe Jr. wrote:
> By rights, we should be moving apr_support.h out of include/, that was a first
> mistake. I have the same understanding as you, Mladen, but the authors of the
> httpd mod_reqtimeout seem to be reaching their fingers into this header.
mod_proxy_fdpass does as well. So maybe we should make it public.
Regards
Rüdiger
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by "William A. Rowe Jr." <wr...@apache.org>.
By rights, we should be moving apr_support.h out of include/, that was a first
mistake. I have the same understanding as you, Mladen, but the authors of the
httpd mod_reqtimeout seem to be reaching their fingers into this header.
On 4/7/2010 6:29 AM, Mladen Turk wrote:
> Hi,
>
> Will you do something about that or do I have
> to make the official -1?
>
> So far you didn't even consider to reply which
> is I presume caused by the email noise.
>
>
> On 04/04/2010 05:58 PM, Mladen Turk wrote:
>> On 04/03/2010 05:43 PM, bjh@apache.org wrote:
>>> Author: bjh
>>> Date: Sat Apr 3 15:43:00 2010
>>> New Revision: 930533
>>>
>>> URL: http://svn.apache.org/viewvc?rev=930533&view=rev
>>> Log:
>>> Add missing APR_DECLARE for apr_wait_for_io_or_timeout() declaration.
>>>
>>
>> I don't think this was missing.
>> apr_wait_for_io_or_timeout was not a public API which you
>> have just made.
>>
>> See what apr_support.h says:
>> @defgroup apr_support Internal APR support functions
>>
>
> Regards
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, Apr 7, 2010 at 9:58 AM, Mladen Turk <mt...@apache.org> wrote:
> On 04/07/2010 03:40 PM, Brian Havard wrote:
>>
>> I'm not sure why but I didn't get your original email quoted below.
>>
>> I added the APR_DECLARE because I can't build httpd trunk without it.
>> apr_wait_for_io_or_timeout() is used in 2 places in httpd. The one
>> causing me trouble is in mod_reqtimeout.c. The other is
>> mod_proxy_fdpass.c but I don't have that enabled.
>>
>> I'm happy to reverse this if that's the right thing to do but httpd will
>> have to remove all references to the function.
>>
>
> I don't have a problem with making that API as public,
> but it shouldn't be declared inside the apr_support.h
> in that case.
> This header is for APR internal declarations only.
My 2 cents:
* apr_support.h shouldn't exist, at least in the main include directory
* apr_wait_for_io_or_timeout() shouldn't be considered public in APR 2.0
** yank it from public headers and let consuming projects adjust their
code (or they can offer an appropriate API that isn't a quick&dirty
interface for internal use)
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Mladen Turk <mt...@apache.org>.
On 04/07/2010 03:40 PM, Brian Havard wrote:
> I'm not sure why but I didn't get your original email quoted below.
>
> I added the APR_DECLARE because I can't build httpd trunk without it.
> apr_wait_for_io_or_timeout() is used in 2 places in httpd. The one
> causing me trouble is in mod_reqtimeout.c. The other is
> mod_proxy_fdpass.c but I don't have that enabled.
>
> I'm happy to reverse this if that's the right thing to do but httpd will
> have to remove all references to the function.
>
I don't have a problem with making that API as public,
but it shouldn't be declared inside the apr_support.h
in that case.
This header is for APR internal declarations only.
Not sure what would be the optimal header for
declaring the public apr_wait_for_io_or_timeout
but apr_network_io.h is the logical choice since
it already has function that operate on apr_file_t and
apr_socket_t
Regards
--
^TM
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Brian Havard <br...@gmail.com>.
I'm not sure why but I didn't get your original email quoted below.
I added the APR_DECLARE because I can't build httpd trunk without it.
apr_wait_for_io_or_timeout() is used in 2 places in httpd. The one
causing me trouble is in mod_reqtimeout.c. The other is
mod_proxy_fdpass.c but I don't have that enabled.
I'm happy to reverse this if that's the right thing to do but httpd will
have to remove all references to the function.
Mladen Turk wrote:
> Hi,
>
> Will you do something about that or do I have
> to make the official -1?
>
> So far you didn't even consider to reply which
> is I presume caused by the email noise.
>
>
> On 04/04/2010 05:58 PM, Mladen Turk wrote:
>> On 04/03/2010 05:43 PM, bjh@apache.org wrote:
>>> Author: bjh
>>> Date: Sat Apr 3 15:43:00 2010
>>> New Revision: 930533
>>>
>>> URL: http://svn.apache.org/viewvc?rev=930533&view=rev
>>> Log:
>>> Add missing APR_DECLARE for apr_wait_for_io_or_timeout() declaration.
>>>
>>
>> I don't think this was missing.
>> apr_wait_for_io_or_timeout was not a public API which you
>> have just made.
>>
>> See what apr_support.h says:
>> @defgroup apr_support Internal APR support functions
>>
>
> Regards
Re: svn commit: r930533 - /apr/apr/trunk/include/apr_support.h
Posted by Mladen Turk <mt...@apache.org>.
Hi,
Will you do something about that or do I have
to make the official -1?
So far you didn't even consider to reply which
is I presume caused by the email noise.
On 04/04/2010 05:58 PM, Mladen Turk wrote:
> On 04/03/2010 05:43 PM, bjh@apache.org wrote:
>> Author: bjh
>> Date: Sat Apr 3 15:43:00 2010
>> New Revision: 930533
>>
>> URL: http://svn.apache.org/viewvc?rev=930533&view=rev
>> Log:
>> Add missing APR_DECLARE for apr_wait_for_io_or_timeout() declaration.
>>
>
> I don't think this was missing.
> apr_wait_for_io_or_timeout was not a public API which you
> have just made.
>
> See what apr_support.h says:
> @defgroup apr_support Internal APR support functions
>
Regards
--
^TM