"William A. Rowe, Jr." <[EMAIL PROTECTED]> writes:
> Folks, this looks wrong after consideration. If someone is familiar
> with the Linux gcc optimizer, please see my last comments in
>
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14147
>
> I'm starting to feel like the optimizer bit us.
That was the first thing I thought. However, since that loop does not
fit on one screen it took a bit more concentration to see what was
happening. Those gotos make it a little more interesting :)
do {
int temp_slash=0;
if (condition met) {
temp_slash=1;
}
...
minimerge:
...
minimerge2:
if (temp_slash) {
r->filename[--filename_len] = '\0';
}
....
if (matches) {
if (last_walk->matched == sec_ent[sec_idx]) {
....
goto minimerge; XXXXXXXXX
}
}
} while (thisinfo.filetype == APR_DIR);
The line marked XXXXXXXX is one place where we go back to a point
before zapping the last char of r->filename without clearing
temp_slash. And since filename_len is decremented, we will zap a
different char the next time.
I didn't check if there are other places where can can go back earlier
in the loop without temp_slash being cleared. But since there are two
goto destinations that seems possible.
It does boggle my mind that we developers apparently never hit this.
I tried to duplicate the mod_dav problem multiple times with no luck.
"William A. Rowe, Jr." <[EMAIL PROTECTED]> writes:
> Folks, this looks wrong after consideration. If someone is familiar
> with the Linux gcc optimizer, please see my last comments in
>
> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=14147
>
> I'm starting to feel like the optimizer bit us.
>
> Bill
> > Index: request.c
> > ===================================================================
> > RCS file: /home/cvs/httpd-2.0/server/request.c,v
> > retrieving revision 1.117
> > retrieving revision 1.118
> > diff -u -r1.117 -r1.118
> > --- request.c 25 Oct 2002 16:38:11 -0000 1.117
> > +++ request.c 1 Nov 2002 03:27:20 -0000 1.118
> > @@ -924,6 +924,8 @@
> > /* That temporary trailing slash was useful, now drop it.
> > */
> > if (temp_slash) {
> > + temp_slash = 0;
> > + AP_ASSERT(r->filename[filename_len-1] == '/');
By the way... I would like to trust this code enough to make it
AP_DEBUG_ASSERT(), but I guess if you're scared I should be scared too
:)
> > r->filename[--filename_len] = '\0';
--
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...
