You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ryan Bloom <rb...@raleigh.ibm.com> on 1999/09/13 13:10:23 UTC
Re: cvs commit: apache-2.0/src/lib/apr/file_io/unix open.c
>
> - if ((*new)->filedes < 0 && (*new)->filehand == NULL) {
> + if ((*new)->filedes < 0 || (*new)->filehand == NULL) {
> (*new)->filedes = -1;
> (*new)->eof_hit = 1;
> return errno;
This is wrong. This says that we NEVER open the file correctly. This is
because we only ever set one of the two options, either the filedes or the
filehand, NEVER both. We are checking for the error condition, not the
successful case. Either the filedes or the filehand WILL be not set after
each open. I have serious doubts that the code even works anymore. If
it does, it is only because we aren't checking return code properly.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/file_io/unix open.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> Yeah but filedes == 0 when you use filehand (which is possibly a bug).
OK. This is the bug. Filedes should be -1. I don't know how I missed
that in my testing, because we are using filedes all over the place. I'll
fix this ASAP.
> OK, well it didn't work before, and it now does _when_ it is using
> filehand. I agree this it is still broken, though, but less so than
> before.
Agreed. The "best" way to fix it, IMHO, is to init filedes to -1 and make
the original check.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/file_io/unix open.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> Yeah but filedes == 0 when you use filehand (which is possibly a bug).
OK. This is the bug. Filedes should be -1. I don't know how I missed
that in my testing, because we are using filedes all over the place. I'll
fix this ASAP.
> OK, well it didn't work before, and it now does _when_ it is using
> filehand. I agree this it is still broken, though, but less so than
> before.
Agreed. The "best" way to fix it, IMHO, is to init filedes to -1 and make
the original check.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/file_io/unix open.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Ryan Bloom wrote:
>
> >
> > - if ((*new)->filedes < 0 && (*new)->filehand == NULL) {
> > + if ((*new)->filedes < 0 || (*new)->filehand == NULL) {
> > (*new)->filedes = -1;
> > (*new)->eof_hit = 1;
> > return errno;
>
> This is wrong. This says that we NEVER open the file correctly. This is
> because we only ever set one of the two options, either the filedes or the
> filehand, NEVER both.
Yeah but filedes == 0 when you use filehand (which is possibly a bug).
> We are checking for the error condition, not the
> successful case. Either the filedes or the filehand WILL be not set after
> each open. I have serious doubts that the code even works anymore. If
> it does, it is only because we aren't checking return code properly.
OK, well it didn't work before, and it now does _when_ it is using
filehand. I agree this it is still broken, though, but less so than
before.
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
- Indira Gandhi
Re: cvs commit: apache-2.0/src/lib/apr/file_io/unix open.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Ryan Bloom wrote:
>
> >
> > - if ((*new)->filedes < 0 && (*new)->filehand == NULL) {
> > + if ((*new)->filedes < 0 || (*new)->filehand == NULL) {
> > (*new)->filedes = -1;
> > (*new)->eof_hit = 1;
> > return errno;
>
> This is wrong. This says that we NEVER open the file correctly. This is
> because we only ever set one of the two options, either the filedes or the
> filehand, NEVER both.
Yeah but filedes == 0 when you use filehand (which is possibly a bug).
> We are checking for the error condition, not the
> successful case. Either the filedes or the filehand WILL be not set after
> each open. I have serious doubts that the code even works anymore. If
> it does, it is only because we aren't checking return code properly.
OK, well it didn't work before, and it now does _when_ it is using
filehand. I agree this it is still broken, though, but less so than
before.
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
- Indira Gandhi
Re: cvs commit: apache-2.0/src/lib/apr/file_io/unix open.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Ryan Bloom wrote:
>
> What platform? On Linux 2.2 I can serve 1000000 (yes, I did run ab with
> that number) just fine. On Linux 2.0, I can serve all the pages I want,
> but I am seeing a core dump every one in a while. I have not had the time
> to track it down.
FreeBSD 2.2.8. I am using a slightly non-trivial config, though it isn't
huge.
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
- Indira Gandhi
Re: cvs commit: apache-2.0/src/lib/apr/file_io/unix open.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
What platform? On Linux 2.2 I can serve 1000000 (yes, I did run ab with
that number) just fine. On Linux 2.0, I can serve all the pages I want,
but I am seeing a core dump every one in a while. I have not had the time
to track it down.
Ryan
On Mon, 13 Sep 1999, Ben Laurie wrote:
> Ryan Bloom wrote:
> >
> > >
> > > - if ((*new)->filedes < 0 && (*new)->filehand == NULL) {
> > > + if ((*new)->filedes < 0 || (*new)->filehand == NULL) {
> > > (*new)->filedes = -1;
> > > (*new)->eof_hit = 1;
> > > return errno;
> >
> > This is wrong. This says that we NEVER open the file correctly. This is
> > because we only ever set one of the two options, either the filedes or the
> > filehand, NEVER both. We are checking for the error condition, not the
> > successful case. Either the filedes or the filehand WILL be not set after
> > each open. I have serious doubts that the code even works anymore. If
> > it does, it is only because we aren't checking return code properly.
>
> BTW, after all the patches I did yesterday, I get an Apache that doesn't
> instantly die, and will accept a request (though it ditches the response
> for some reason as yet undiagnosed).
>
> All the non-warning patches fixed a bug, though possibly not in the best
> way.
>
> Cheers,
>
> Ben.
>
> --
> http://www.apache-ssl.org/ben.html
>
> "My grandfather once told me that there are two kinds of people: those
> who work and those who take the credit. He told me to try to be in the
> first group; there was less competition there."
> - Indira Gandhi
>
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/file_io/unix open.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Ryan Bloom wrote:
>
> >
> > - if ((*new)->filedes < 0 && (*new)->filehand == NULL) {
> > + if ((*new)->filedes < 0 || (*new)->filehand == NULL) {
> > (*new)->filedes = -1;
> > (*new)->eof_hit = 1;
> > return errno;
>
> This is wrong. This says that we NEVER open the file correctly. This is
> because we only ever set one of the two options, either the filedes or the
> filehand, NEVER both. We are checking for the error condition, not the
> successful case. Either the filedes or the filehand WILL be not set after
> each open. I have serious doubts that the code even works anymore. If
> it does, it is only because we aren't checking return code properly.
BTW, after all the patches I did yesterday, I get an Apache that doesn't
instantly die, and will accept a request (though it ditches the response
for some reason as yet undiagnosed).
All the non-warning patches fixed a bug, though possibly not in the best
way.
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
- Indira Gandhi