Re: Endless loop in split_on_bdry() of library/parser_multipart.c?

[Bojan Smojver]

On Wed, 2008-06-04 at 21:35 -0700, Joe Schaefer wrote:

 Needs a comment in the source about why we're using
 volatile here, but otherwise +1.

Done on both the trunk and v2_10 branch.

Any comment regarding my other patch about strict aliasing warnings?

-- 
Bojan

Re: Endless loop in split_on_bdry() of library/parser_multipart.c?

[Bojan Smojver]

I propose we fix this as attached. I tested this on Fedora 9 and it
works OK. Opinions?

-- 
Bojan
Index: library/parser_multipart.c
===
--- library/parser_multipart.c	(revision 663420)
+++ library/parser_multipart.c	(working copy)
@@ -162,11 +162,13 @@
  * so we can move previous buckets across
  * and retest buf against the full bdry.
  */
+apr_bucket_brigade * volatile in_v = in;
+
 do {
-apr_bucket *f = APR_BRIGADE_FIRST(in);
+apr_bucket *f = APR_BRIGADE_FIRST(in_v);
 APR_BUCKET_REMOVE(f);
 APR_BRIGADE_INSERT_TAIL(out, f);
-} while (e != APR_BRIGADE_FIRST(in));
+} while (e != APR_BRIGADE_FIRST(in_v));
 off = 0;
 goto look_for_boundary_up_front;
 }
Index: acinclude.m4
===
--- acinclude.m4	(revision 663420)
+++ acinclude.m4	(working copy)
@@ -214,7 +214,6 @@
   ])
 # -Wdeclaration-after-statement is only supported on gcc 3.4+
 fi
-APR_ADDTO([CFLAGS], -fno-strict-aliasing)
 
 APR_ADDTO([CPPFLAGS], `$APR_CONFIG --cppflags`)
 

Re: Endless loop in split_on_bdry() of library/parser_multipart.c?

[Joe Schaefer]

--- Bojan Smojver [EMAIL PROTECTED] wrote:

 I propose we fix this as attached. I tested this on
 Fedora 9 and it
 works OK. Opinions?

Needs a comment in the source about why we're using
volatile here, but otherwise +1.



  

Re: Endless loop in split_on_bdry() of library/parser_multipart.c?

[Bojan Smojver]

Quoting Bojan Smojver [EMAIL PROTECTED]:


Looks like the offsetof() provided by the platform isn't being used.
Which in turn causes a lot of casting all over the place, which
creates the aliasing problem? Maybe?


Nah, it isn't that... Fails just the same with native offsetof() :-(

--
Bojan

Re: Endless loop in split_on_bdry() of library/parser_multipart.c?

[Bojan Smojver]

On Sun, 2006-06-11 at 20:31 -0400, Joe Schaefer wrote:

APR_RING_UNSPLICE(f, l, link);
APR_RING_SPLICE_TAIL(out-list, f, l, apr_bucket, link);
 
 
 This is the right approach, I think.  But the person who'd be
 in the best place to test/commit it is Bojan.  Just be sure
 to bump the patch level in apreq_version.h, and add a comment
 to CHANGES.

I can test the above in my setup and see what comes out, but I'm more
worried about the incorrectly compiled code and why it happened. I'm not
sure if it's due to bugs in gcc, or is it something that RING/BRIGADE
macros are doing that really isn't quite right.

RH gcc guys know about it and they already put a few comments into the
bug report, but I'm guessing they are still working on a final verdict.

In any event, we know how to work around the issue on the platform in
question (Fedora Core), so the whole thing is under control. Well, sort
of anyway... :-)

-- 
Bojan

Re: Endless loop in split_on_bdry() of library/parser_multipart.c?

[Bojan Smojver]

Quoting Philip M. Gollucci [EMAIL PROTECTED]:


Seems to be Fedora Core X specific.


Happens on x84_64 as well and with 2.07. Rebuilding the package in  
Fedora Extras 5 now.


--
Bojan

Re: Endless loop in split_on_bdry() of library/parser_multipart.c?

[Bojan Smojver]

Quoting Joe Schaefer [EMAIL PROTECTED]:


At least now it's a bit clearer why the no-strict-aliasing
optimization is getting confused ;-)


Hey, speak for yourself ;-)

--
Bojan