You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Stein <gs...@lyra.org> on 2000/11/04 00:09:31 UTC
Re: cvs commit: apache-2.0/src/lib/apr/test testsf.c
On Fri, Nov 03, 2000 at 06:05:11PM -0000, trawick@locus.apache.org wrote:
> trawick 00/11/03 10:05:10
>
> Modified: src/lib/apr/include apr_network_io.h
> src/lib/apr/network_io/unix sendrecv.c
> src/lib/apr/network_io/win32 sendrecv.c
> src/lib/apr/test testsf.c
> Log:
> Make the len parm to apr_sendfile() apr_ssize_t * instead of apr_size_t *
> for consistency with other APR network send/recv calls.
This isn't right. The apr_ssize_t implies that a negative number can be
returned. That isn't the case is it? If not, then this should be reverted.
The parameters should follow the semantic model, NOT follow something else
simply for consistency.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/test testsf.c
Posted by Jeff Trawick <tr...@bellsouth.net>.
Greg Stein <gs...@lyra.org> writes:
> On Mon, Nov 06, 2000 at 02:40:49PM -0800, rbb@covalent.net wrote:
> >...
> > Anyway, that's the historical reason for everything, and it should all
> > change, BUT it shouldn't change until the APR mailing lists are created,
> > even if the code isn't split immediately. The reason for the wait, is
> > that this is a large change to the API, and I would like other projects to
> > have the chance to chime in on it. Plus, this is a good way to start to
> > migrate talk about APR off new-httpd.
>
> If it should change, then it should just change. No need to wait on these
> things. In this particular case, it isn't going to affect SVN. A warning
> here or there is easy to clean up.
I am happy to do this work and I'm not picky about when; at such time
that it becomes obvious that nobody is going to be pissed off at the
timing I'll move ahead with it. If somebody is itching to do it
themselves, speak up.
--
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: cvs commit: apache-2.0/src/lib/apr/test testsf.c
Posted by Greg Stein <gs...@lyra.org>.
On Mon, Nov 06, 2000 at 02:40:49PM -0800, rbb@covalent.net wrote:
>...
> Anyway, that's the historical reason for everything, and it should all
> change, BUT it shouldn't change until the APR mailing lists are created,
> even if the code isn't split immediately. The reason for the wait, is
> that this is a large change to the API, and I would like other projects to
> have the chance to chime in on it. Plus, this is a good way to start to
> migrate talk about APR off new-httpd.
If it should change, then it should just change. No need to wait on these
things. In this particular case, it isn't going to affect SVN. A warning
here or there is easy to clean up.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/test testsf.c
Posted by rb...@covalent.net.
> > This isn't right. The apr_ssize_t implies that a negative number can be
> > returned. That isn't the case is it? If not, then this should be
> > reverted.
>
> Rather than simply revert it, I'd say change everything to apr_size_t
> and deal with the consequences. It doesn't make sense to simply back
> this out and be left with the strange inconsistency.
>
> *But first*, why were the other functions apr_ssize_t instead of
> apr_size_t? It seems likely that it was intentional.
This was intentional. It is also outdated. Everything should change to
apr_size_t now. The original reason for it, was that we would return -1
in the length parameter if there was an error. This was later decided to
be stupid, because we had a status value to indicate errors, and we had
written 0 bytes, so we just returned 0 as we should have.
Anyway, that's the historical reason for everything, and it should all
change, BUT it shouldn't change until the APR mailing lists are created,
even if the code isn't split immediately. The reason for the wait, is
that this is a large change to the API, and I would like other projects to
have the chance to chime in on it. Plus, this is a good way to start to
migrate talk about APR off new-httpd.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apache-2.0/src/lib/apr/test testsf.c
Posted by Jeff Trawick <tr...@bellsouth.net>.
Greg Stein <gs...@lyra.org> writes:
> On Fri, Nov 03, 2000 at 06:05:11PM -0000, trawick@locus.apache.org wrote:
> > trawick 00/11/03 10:05:10
> >
> > Modified: src/lib/apr/include apr_network_io.h
> > src/lib/apr/network_io/unix sendrecv.c
> > src/lib/apr/network_io/win32 sendrecv.c
> > src/lib/apr/test testsf.c
> > Log:
> > Make the len parm to apr_sendfile() apr_ssize_t * instead of apr_size_t *
> > for consistency with other APR network send/recv calls.
>
> This isn't right. The apr_ssize_t implies that a negative number can be
> returned. That isn't the case is it? If not, then this should be
> reverted.
Rather than simply revert it, I'd say change everything to apr_size_t
and deal with the consequences. It doesn't make sense to simply back
this out and be left with the strange inconsistency.
*But first*, why were the other functions apr_ssize_t instead of
apr_size_t? It seems likely that it was intentional.
I have seen the use of signed integers justified on this mailing list
by the slight speed advantage of signed arithmetic on some (most?)
hardware. In some software I was involved in on S/390, somebody
changed a number of unsigned fields to signed fields for a measurable
performance improvement.
> The parameters should follow the semantic model, NOT follow something else
> simply for consistency.
Both have their importance. As far as following the semantic model
with variables, I've had you tell me to use a signed 32-bit integer
for something that has only two values :) There are plenty of
examples where signed ints are used in preference to whatever most
closely matches the semantic model.
I doubt that there is any one correct answer here (unless of course on
a 32-bit machine we expect to pass 2+GB pieces of storage to an APR
send/recv function).
Mostly serious,
Jeff
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...