Paul Burba wrote:
>
> [[[
> OS400/EBCDIC Port: apr_poll() signature differences.
>
> This is the third of several patches to allow Subversion to run on IBM's
> OS400 V5R4.
>
> IBM's implementation of apr_poll() requires a pool argument. We have also
> experienced unpredictable failures with apr_poll() when the nsds parameter
> is uninitialized.
>
> * subversion/libsvn_ra_svn/marshal.c
> (svn_ra_svn__input_waiting):
> * subversion/libsvn_subr/prompt.c
> (wait_for_input):
> Initialize nsds parameter and supply pool argument for
> apr_poll() calls.
> ]]]
>
> Index: subversion/libsvn_ra_svn/marshal.c
> ===================================================================
> --- subversion/libsvn_ra_svn/marshal.c (revision 18448)
> +++ subversion/libsvn_ra_svn/marshal.c (working copy)
> @@ -121,7 +121,15 @@
> }
> pfd.p = pool;
> pfd.reqevents = APR_POLLIN;
> +#ifndef AS400
> return ((apr_poll(&pfd, 1, &n, 0) == APR_SUCCESS) && n);
> +#else
> + /* IBM's apr_poll() implmentation behaves badly with some large values
Typo: "implementation" (in both files).
> + * of n (apr_palloc fails) so we initialize it. */
> + n = 0;
That sounds like a trivial bug. Will IBM soon release a version of APR with
this bug fixed? In that case we wouldn't need this work-around. The
doc-string of apr_poll ought to be improved to say that this is an output
parameter; it's a bit vague presently.
> + /* OS400 requires a pool argument for apr_poll(). */
> + return ((apr_poll(&pfd, 1, &n, 0, pool) == APR_SUCCESS) && n);
> +#endif
It would make sense for mainstream APR to revise this API, adding the pool
parameter. Do you know if IBM has proposed that to the APR developers (or are
likely to)? Not that we will be able to rely on it for a long time yet, but it
should happen anyway.
> }
>
> /* --- WRITE BUFFER MANAGEMENT --- */
> Index: subversion/libsvn_subr/prompt.c
> ===================================================================
> --- subversion/libsvn_subr/prompt.c (revision 18448)
> +++ subversion/libsvn_subr/prompt.c (working copy)
> @@ -59,7 +59,15 @@
The doc string of this function (wait_for_input) is pretty unintelligible, but
it's not for you to fix it.
(If anyone wants a (miserable) laugh, take a look at the doc string for
apr_wait_for_io_or_timeout():
/**
* Wait for IO to occur or timeout.
*
* Uses POOL for temporary allocations.
*/
apr_status_t apr_wait_for_io_or_timeout(apr_file_t *f, apr_socket_t *s,
int for_read);
What a pile of poo! Somebody's not been reading what they're writing.
OK, I suppose I ought not to ridicule it here but report it to the proper
authority.)
> pollset.p = pool;
> pollset.reqevents = APR_POLLIN;
>
> +#ifndef AS400
> srv = apr_poll(&pollset, 1, &n, -1);
> +#else
> + /* IBM's apr_poll() implmentation behaves badly with some large values
> + * of n (apr_palloc fails) so we initialize it. */
> + n = 0;
> + /* OS400 requires a pool argument for apr_poll(). */
> + srv = apr_poll(&pollset, 1, &n, -1, pool);
> +#endif
>
Well, honestly, yuck, but as it's only twice, and doesn't touch any of our
APIs, I could accept this patch as it is.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Feb 13 23:32:16 2006