[Fwd: connobject.c broke with apr 1.3.2]

[William A. Rowe, Jr.]

Guys, please take a look, looks like one of the 1.3.x generation patches
has broken our API compatibility rules.


Hi,

  Upgrading apr to the latest 1.3.2 broke the compilation of mod_python (I use
mod_python 3.3.1 but this file is the same in svn). Since b is a bucket and bb
the bucket brigade object, I suppose we must read APR_BRIGADE_SENTINEL(bb)
instead of APR_BRIGADE_SENTINEL(b). Any APR lib expert confirmation ?

diff -r -u -p mod_python-3.3.1.orig/src/connobject.c
mod_python-3.3.1/src/connobject.c
--- mod_python-3.3.1.orig/src/connobject.c      2008-06-25 16:16:41 +0200
+++ mod_python-3.3.1/src/connobject.c   2008-06-25 16:20:32 +0200
@@ -139,7 +139,7 @@ static PyObject * _conn_read(conn_rec *c
     bytes_read = 0;

     while ((bytes_read < len || len == 0) &&
-           !(b == APR_BRIGADE_SENTINEL(b) ||
+           !(b == APR_BRIGADE_SENTINEL(bb) ||
              APR_BUCKET_IS_EOS(b) || APR_BUCKET_IS_FLUSH(b))) {

         const char *data;

--
  Cyrille

[Tom Donovan-2]

William A. Rowe, Jr. wrote:
> Guys, please take a look, looks like one of the 1.3.x generation patches
> has broken our API compatibility rules.
>
It looks like this has always been wrong in mod_python.

APR_BRIGADE_SENTINEL takes a brigade argument, not a bucket.

refs:
http://apr.apache.org/docs/apr-util/1.3/group___a_p_r___util___bucket___brigades.html#g858da66dccab1e063415678bb115788a
http://apr.apache.org/docs/apr-util/0.9/group__APR__Util__Bucket__Brigades.html#g858da66dccab1e063415678bb115788a


I think no error displayed pre-1.3.2 because, unfortunately, both buckets and brigades have a member
named 'list':

    For buckets - 'list' is a pointer to the allocator function.
    For brigades, 'list' is the ring of buckets.

The condition was probably never true when he was checking the wrong kind of 'list' for a brigade
sentinel.  Well, hopefully it was never true!

-tom-

[Ruediger Pluem]

On 06/30/2008 08:28 PM, Tom Donovan wrote:
> William A. Rowe, Jr. wrote:
>> Guys, please take a look, looks like one of the 1.3.x generation patches
>> has broken our API compatibility rules.
>>
> It looks like this has always been wrong in mod_python.
>
> APR_BRIGADE_SENTINEL takes a brigade argument, not a bucket.

I agree. This has been the case for as long as I can remember and svn blame
proves that this has been this way for ages:

http://svn.apache.org/viewvc?view=rev&revision=58049

>
> refs:
> http://apr.apache.org/docs/apr-util/1.3/group___a_p_r___util___bucket___brigades.html#g858da66dccab1e063415678bb115788a 
>
> http://apr.apache.org/docs/apr-util/0.9/group__APR__Util__Bucket__Brigades.html#g858da66dccab1e063415678bb115788a 
>
>
>
> I think no error displayed pre-1.3.2 because, unfortunately, both
> buckets and brigades have a member named 'list':
>
>    For buckets - 'list' is a pointer to the allocator function.
>    For brigades, 'list' is the ring of buckets.
>
> The condition was probably never true when he was checking the wrong
> kind of 'list' for a brigade sentinel.  Well, hopefully it was never true!

Good points.

Regards

Rüdiger

[Bojan Smojver]

On Mon, 2008-06-30 at 12:17 -0500, William A. Rowe, Jr. wrote:
> Guys, please take a look, looks like one of the 1.3.x generation patches
> has broken our API compatibility rules.

I'll bet $5 that this broke it:

http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_ring.h?r1=566349&r2=662299

However, calling APR_BRIGADE_SENTINEL on a bucket is the wrong thing to
do, so I think mod_python folks should fix their code.

--
Bojan

[Bojan Smojver]

On Tue, 2008-07-01 at 07:26 +1000, Bojan Smojver wrote:

> I'll bet $5 that this broke it:

Yep, GCC will say:

error: request for member 'next' in something not a structure or union

--
Bojan

[Bojan Smojver]

On Tue, 2008-07-01 at 07:26 +1000, Bojan Smojver wrote:

> http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_ring.h?r1=566349&r2=662299

BTW, this calls for docs regeneration, so that people can see what that
macro actually does. I'll do that once we release 1.3.3.

--
Bojan