You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jon Travis <jt...@covalent.net> on 2000/04/03 22:55:24 UTC
PATCH: APR buffered I/O
Attached are some patches to get ungetc() working in APR for
non-buffered
file descriptors. In addition, I believe I fixed a bug with the
ap_fgets() under
Unix (it looks like Win32 has it correct). I have tested out all these
changes,
but please poke a careful eye into it anyway.. ;-)
Also, I noticed that ap_seek() has arguments in reverse order from what
UNIX
says that they should be. Perhaps we should change this for consistancy
sake?
-- Jon
Re: PATCH: APR buffered I/O
Posted by Jon Travis <jt...@covalent.net>.
Jeff Trawick wrote:
> The unix one looks goofy too. Where is the string terminated
> for non-buffered files? Was that in the patch too?
>
> --
> Jeff Trawick | trawick@ibm.net | PGP public key at web site:
> http://www.geocities.com/SiliconValley/Park/9289/
> Born in Roswell... married an alien...
Yes.. ;-)
"In addition, I believe I fixed a bug with the ap_fgets() under Unix"
I should have made more clear which one that was... I.e. aside
from the bug of non-buffered unget stuffs.
-- Jon
Re: PATCH: APR buffered I/O
Posted by Jeff Trawick <tr...@bellsouth.net>.
The unix one looks goofy too. Where is the string terminated
for non-buffered files? Was that in the patch too?
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: PATCH: APR buffered I/O
Posted by Jeff Trawick <tr...@bellsouth.net>.
str[i] = '\0'; (when i == len, of course)
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: PATCH: APR buffered I/O
Posted by Bill Stoddard <st...@raleigh.ibm.com>.
> Heh -- I just can't win with any patches!.. ;-) Make sure you fix up the
> ap_fgets() routine though. After taking a quick glance, it looks like the
> win32 one is also broken (it can write off the end of the buffer)..
>
Looks okay to me.
Bill
Re: PATCH: APR buffered I/O
Posted by Jon Travis <jt...@covalent.net>.
rbb@apache.org wrote:
> On Mon, 3 Apr 2000, Manoj Kasichainula wrote:
>
> > On Mon, Apr 03, 2000 at 05:35:15PM -0400, Ryan Bloom wrote:
> > > This patch brings up an interesting question. Do we even want to support
> > > buffered I/O in APR? We supported it originally because Apache needed it
> > > originally. Since then, Apache has removed ALL buffered I/O support, and
> > > the support in APR has fallen by the way-side.
> >
> > +1 on removing anything depending on stdio I/O calls.
>
> That's three +1's. I'll give this another day or two to let other people
> pipe in, and then the stdio I/O stuff is blown out of APR.
>
> Ryan
Heh -- I just can't win with any patches!.. ;-) Make sure you fix up the
ap_fgets() routine though. After taking a quick glance, it looks like the
win32 one is also broken (it can write off the end of the buffer)..
-- Jon
Re: PATCH: APR buffered I/O
Posted by rb...@apache.org.
On Mon, 3 Apr 2000, Manoj Kasichainula wrote:
> On Mon, Apr 03, 2000 at 05:35:15PM -0400, Ryan Bloom wrote:
> > This patch brings up an interesting question. Do we even want to support
> > buffered I/O in APR? We supported it originally because Apache needed it
> > originally. Since then, Apache has removed ALL buffered I/O support, and
> > the support in APR has fallen by the way-side.
>
> +1 on removing anything depending on stdio I/O calls.
That's three +1's. I'll give this another day or two to let other people
pipe in, and then the stdio I/O stuff is blown out of APR.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: PATCH: APR buffered I/O
Posted by Manoj Kasichainula <ma...@io.com>.
On Mon, Apr 03, 2000 at 05:35:15PM -0400, Ryan Bloom wrote:
> This patch brings up an interesting question. Do we even want to support
> buffered I/O in APR? We supported it originally because Apache needed it
> originally. Since then, Apache has removed ALL buffered I/O support, and
> the support in APR has fallen by the way-side.
+1 on removing anything depending on stdio I/O calls.
Re: PATCH: APR buffered I/O
Posted by Manoj Kasichainula <ma...@io.com>.
On Mon, Apr 03, 2000 at 05:57:37PM -0700, Greg Stein wrote:
> If we have no use for it now, then it should go.
> If we need it in the future, then we can re-examine what to do.
> If we believe BUFF is the "right" answer, then we can discuss packaging.
While I wholeheartedly agree with everything Greg said in this note,
the biggest reason to throw out the buffered I/O support is that it
relies on stdio, and the Solaris stdio implementation only allows 256
FILE *s.
Oh, and it makes the code far more cruftier.
Re: PATCH: APR buffered I/O
Posted by Greg Stein <gs...@lyra.org>.
Not at all! :-)
If we have no use for it now, then it should go.
If we need it in the future, then we can re-examine what to do.
If we believe BUFF is the "right" answer, then we can discuss packaging.
:-)
IMO, we keep APR as the "portable" stuff, and package a (new) "libap.a"
for useful utilities.
Note this also follows Bill's point: don't implement for an unknown
future. It will certainly change by that point, so all you're doing is
creating useless work today. In other words: don't build/design a
buffering stuff until it is needed. Rip it out now, come back later.
Cheers,
-g
On Mon, 3 Apr 2000, David Reid wrote:
> So now you want to reopen the discussion about what belongs in APR again???
>
> :-))
>
> d.
> ----- Original Message -----
> From: "Greg Stein" <gs...@lyra.org>
> To: <ne...@apache.org>
> Sent: Monday, April 03, 2000 10:46 PM
> Subject: Re: PATCH: APR buffered I/O
>
>
> > I'm all for nuking buffered support. Why keep around (and maintain!) what
> > isn't needed?
> >
> > [shades of proxy support arise...]
> >
> > The only factor for keeping it in would be the hypothetical future clients
> > of APR. Would they *need* the support? Beats the crap outta me. I would
> > think "probably". However, a better solution than buffered files may be
> > bundling BUFF.
> >
> > Cheers,
> > -g
> >
> > On Mon, 3 Apr 2000 rbb@apache.org wrote:
> > > This patch brings up an interesting question. Do we even want to
> support
> > > buffered I/O in APR? We supported it originally because Apache needed
> it
> > > originally. Since then, Apache has removed ALL buffered I/O support,
> and
> > > the support in APR has fallen by the way-side.
> > >
> > > Just asking.
> > >
> > > Ryan
> > >
> > > On Mon, 3 Apr 2000, Greg Stein wrote:
> > >
> > > > Watch out for sign extension in ap_ungetc() !!
> > > >
> > > > It is quite possible that somebody passes in \xFF and the assignment
> to
> > > > ->ungetchar will sign-extend to -1.
> > > >
> > > > The assignment should look something like:
> > > >
> > > > thefile->ungetchar = (unsigned char)ch;
> > > >
> > > > Cheers,
> > > > -g
> > > >
> > > >
> > > > On Mon, 3 Apr 2000, Jon Travis wrote:
> > > >
> > > > > Attached are some patches to get ungetc() working in APR for
> > > > > non-buffered
> > > > > file descriptors. In addition, I believe I fixed a bug with the
> > > > > ap_fgets() under
> > > > > Unix (it looks like Win32 has it correct). I have tested out all
> these
> > > > > changes,
> > > > > but please poke a careful eye into it anyway.. ;-)
> > > > >
> > > > > Also, I noticed that ap_seek() has arguments in reverse order from
> what
> > > > > UNIX
> > > > > says that they should be. Perhaps we should change this for
> consistancy
> > > > > sake?
> > > > >
> > > > > -- Jon
> > > > >
> > > > >
> > > > >
> > > >
> > > > --
> > > > Greg Stein, http://www.lyra.org/
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> ____________________________________________________________________________
> ___
> > > Ryan Bloom rbb@apache.org
> > > 406 29th St.
> > > San Francisco, CA 94131
> >
> > --------------------------------------------------------------------------
> -----
> > >
> >
> > --
> > Greg Stein, http://www.lyra.org/
> >
> >
>
--
Greg Stein, http://www.lyra.org/
Re: PATCH: APR buffered I/O
Posted by David Reid <dr...@jetnet.co.uk>.
So now you want to reopen the discussion about what belongs in APR again???
:-))
d.
----- Original Message -----
From: "Greg Stein" <gs...@lyra.org>
To: <ne...@apache.org>
Sent: Monday, April 03, 2000 10:46 PM
Subject: Re: PATCH: APR buffered I/O
> I'm all for nuking buffered support. Why keep around (and maintain!) what
> isn't needed?
>
> [shades of proxy support arise...]
>
> The only factor for keeping it in would be the hypothetical future clients
> of APR. Would they *need* the support? Beats the crap outta me. I would
> think "probably". However, a better solution than buffered files may be
> bundling BUFF.
>
> Cheers,
> -g
>
> On Mon, 3 Apr 2000 rbb@apache.org wrote:
> > This patch brings up an interesting question. Do we even want to
support
> > buffered I/O in APR? We supported it originally because Apache needed
it
> > originally. Since then, Apache has removed ALL buffered I/O support,
and
> > the support in APR has fallen by the way-side.
> >
> > Just asking.
> >
> > Ryan
> >
> > On Mon, 3 Apr 2000, Greg Stein wrote:
> >
> > > Watch out for sign extension in ap_ungetc() !!
> > >
> > > It is quite possible that somebody passes in \xFF and the assignment
to
> > > ->ungetchar will sign-extend to -1.
> > >
> > > The assignment should look something like:
> > >
> > > thefile->ungetchar = (unsigned char)ch;
> > >
> > > Cheers,
> > > -g
> > >
> > >
> > > On Mon, 3 Apr 2000, Jon Travis wrote:
> > >
> > > > Attached are some patches to get ungetc() working in APR for
> > > > non-buffered
> > > > file descriptors. In addition, I believe I fixed a bug with the
> > > > ap_fgets() under
> > > > Unix (it looks like Win32 has it correct). I have tested out all
these
> > > > changes,
> > > > but please poke a careful eye into it anyway.. ;-)
> > > >
> > > > Also, I noticed that ap_seek() has arguments in reverse order from
what
> > > > UNIX
> > > > says that they should be. Perhaps we should change this for
consistancy
> > > > sake?
> > > >
> > > > -- Jon
> > > >
> > > >
> > > >
> > >
> > > --
> > > Greg Stein, http://www.lyra.org/
> > >
> > >
> > >
> > >
> >
> >
> >
____________________________________________________________________________
___
> > Ryan Bloom rbb@apache.org
> > 406 29th St.
> > San Francisco, CA 94131
>
> --------------------------------------------------------------------------
-----
> >
>
> --
> Greg Stein, http://www.lyra.org/
>
>
Re: PATCH: APR buffered I/O
Posted by Greg Stein <gs...@lyra.org>.
I'm all for nuking buffered support. Why keep around (and maintain!) what
isn't needed?
[shades of proxy support arise...]
The only factor for keeping it in would be the hypothetical future clients
of APR. Would they *need* the support? Beats the crap outta me. I would
think "probably". However, a better solution than buffered files may be
bundling BUFF.
Cheers,
-g
On Mon, 3 Apr 2000 rbb@apache.org wrote:
> This patch brings up an interesting question. Do we even want to support
> buffered I/O in APR? We supported it originally because Apache needed it
> originally. Since then, Apache has removed ALL buffered I/O support, and
> the support in APR has fallen by the way-side.
>
> Just asking.
>
> Ryan
>
> On Mon, 3 Apr 2000, Greg Stein wrote:
>
> > Watch out for sign extension in ap_ungetc() !!
> >
> > It is quite possible that somebody passes in \xFF and the assignment to
> > ->ungetchar will sign-extend to -1.
> >
> > The assignment should look something like:
> >
> > thefile->ungetchar = (unsigned char)ch;
> >
> > Cheers,
> > -g
> >
> >
> > On Mon, 3 Apr 2000, Jon Travis wrote:
> >
> > > Attached are some patches to get ungetc() working in APR for
> > > non-buffered
> > > file descriptors. In addition, I believe I fixed a bug with the
> > > ap_fgets() under
> > > Unix (it looks like Win32 has it correct). I have tested out all these
> > > changes,
> > > but please poke a careful eye into it anyway.. ;-)
> > >
> > > Also, I noticed that ap_seek() has arguments in reverse order from what
> > > UNIX
> > > says that they should be. Perhaps we should change this for consistancy
> > > sake?
> > >
> > > -- Jon
> > >
> > >
> > >
> >
> > --
> > Greg Stein, http://www.lyra.org/
> >
> >
> >
> >
>
>
> _______________________________________________________________________________
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------
>
--
Greg Stein, http://www.lyra.org/
Re: PATCH: APR buffered I/O
Posted by rb...@apache.org.
This patch brings up an interesting question. Do we even want to support
buffered I/O in APR? We supported it originally because Apache needed it
originally. Since then, Apache has removed ALL buffered I/O support, and
the support in APR has fallen by the way-side.
Just asking.
Ryan
On Mon, 3 Apr 2000, Greg Stein wrote:
> Watch out for sign extension in ap_ungetc() !!
>
> It is quite possible that somebody passes in \xFF and the assignment to
> ->ungetchar will sign-extend to -1.
>
> The assignment should look something like:
>
> thefile->ungetchar = (unsigned char)ch;
>
> Cheers,
> -g
>
>
> On Mon, 3 Apr 2000, Jon Travis wrote:
>
> > Attached are some patches to get ungetc() working in APR for
> > non-buffered
> > file descriptors. In addition, I believe I fixed a bug with the
> > ap_fgets() under
> > Unix (it looks like Win32 has it correct). I have tested out all these
> > changes,
> > but please poke a careful eye into it anyway.. ;-)
> >
> > Also, I noticed that ap_seek() has arguments in reverse order from what
> > UNIX
> > says that they should be. Perhaps we should change this for consistancy
> > sake?
> >
> > -- Jon
> >
> >
> >
>
> --
> Greg Stein, http://www.lyra.org/
>
>
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: PATCH: APR buffered I/O
Posted by Greg Stein <gs...@lyra.org>.
Watch out for sign extension in ap_ungetc() !!
It is quite possible that somebody passes in \xFF and the assignment to
->ungetchar will sign-extend to -1.
The assignment should look something like:
thefile->ungetchar = (unsigned char)ch;
Cheers,
-g
On Mon, 3 Apr 2000, Jon Travis wrote:
> Attached are some patches to get ungetc() working in APR for
> non-buffered
> file descriptors. In addition, I believe I fixed a bug with the
> ap_fgets() under
> Unix (it looks like Win32 has it correct). I have tested out all these
> changes,
> but please poke a careful eye into it anyway.. ;-)
>
> Also, I noticed that ap_seek() has arguments in reverse order from what
> UNIX
> says that they should be. Perhaps we should change this for consistancy
> sake?
>
> -- Jon
>
>
>
--
Greg Stein, http://www.lyra.org/
Re: PATCH: APR buffered I/O
Posted by rb...@apache.org.
> Also, I noticed that ap_seek() has arguments in reverse order from what
> UNIX
> says that they should be. Perhaps we should change this for consistancy
> sake?
Didn't we fix this a LONG time ago? I haven't looked at the code yet, I
just seem to recall already having this discussion once.:-)
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------