You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ian Holsman <ia...@cnet.com> on 2001/08/08 07:05:31 UTC

apr_bucket_split : should it take a offset or a size?

should the 'point' parameter in the bucket split (shared/notimpl/ & simple)
be a 'apr_off_t' or a 'apr_size_t' ?

This seems to be the only warning generated from the Win2k build

http_protocol.c
c:\src\apache\httpd-2.0\modules\http\http_protocol.c(678) : warning 
C4244: 'function' : conversion from '__int64 ' to 'unsigned int ', 
possible loss of data

the offending parameter is 'readbytes'

apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, 
ap_input_mode_t mode, apr_off_t *readbytes)


..Ian


Re: apr_bucket_split : should it take a offset or a size?

Posted by Ryan Bloom <rb...@covalent.net>.
That code is incorrect right now.  I am close to re-writing it with apr_brigade_partition,
but that caused some bugs yesterday.  The real problem is that the code in that filter
is trying to do too much, and it isn't taking advantage of the mechanism we provide.

All of the types are correct though.  I can go over the logic behind it if you would like.

Ryan

On Tuesday 07 August 2001 22:05, Ian Holsman wrote:
> should the 'point' parameter in the bucket split (shared/notimpl/ & simple)
> be a 'apr_off_t' or a 'apr_size_t' ?
>
> This seems to be the only warning generated from the Win2k build
>
> http_protocol.c
> c:\src\apache\httpd-2.0\modules\http\http_protocol.c(678) : warning
> C4244: 'function' : conversion from '__int64 ' to 'unsigned int ',
> possible loss of data
>
> the offending parameter is 'readbytes'
>
> apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
> ap_input_mode_t mode, apr_off_t *readbytes)
>
>
> ..Ian

-- 

_____________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
-----------------------------------------------------------------------------

Re: apr_bucket_split : should it take a offset or a size?

Posted by Greg Stein <gs...@lyra.org>.
On Thu, Aug 09, 2001 at 05:05:26PM -0700, Ryan Bloom wrote:
> On Thursday 09 August 2001 15:33, Greg Stein wrote:
> > On Wed, Aug 08, 2001 at 01:40:33AM -0400, Cliff Woolley wrote:
>...
> > > As of late, the rule has become this: individual buckets can contain no
> > > more than an apr_size_t.  A brigade can have total length up to an
> > > apr_off_t.  So apr_bucket_split() and friends SHOULD take an apr_size_t,
> > > because that's the biggest the bucket can be anyway.  Something within
> > > ap_http_filter needs to handle the conversion, I'd expect.
> >
> > FILE, PIPE, and SOCKET buckets can easily be more than "memory-sized".
> > Thus, all of the split and partition functions should take an apr_off_t.
> >
> > When you read, you'll only get back a portion.
> >
> > So... the "rule" you state ought to be changed. It just doesn't apply to
> > certain types of buckets.
> 
> Buckets are buckets.  To have different rules for some buckets than others means that the 
> API's would be inconsistant.  Plus, PIPE and SOCKET buckets always have a -1 length.
> FILE buckets are basically capped at apr_size_t, because having more than that in one
> bucket doesn't really help you anyway.  The argument was that every operating system with
> sendfile we support will do a partial write of the file, because 2Gig is too big to send at one time.
> Without this change, we get all sorts of warnings on platforms where apr_size_t and apr_off_t
> aren't the same.

What?! I'm not saying *some* bucket use one size, and the other buckets
another size. I'm saying all the split functions should be an apr_off_t.

Their content can be larger than apr_size_t, so the split points needs to be
an apr_off_t.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: apr_bucket_split : should it take a offset or a size?

Posted by Ryan Bloom <rb...@covalent.net>.
On Thursday 09 August 2001 15:33, Greg Stein wrote:
> On Wed, Aug 08, 2001 at 01:40:33AM -0400, Cliff Woolley wrote:
> > On Tue, 7 Aug 2001, Ian Holsman wrote:
> > > should the 'point' parameter in the bucket split (shared/notimpl/ &
> > > simple) be a 'apr_off_t' or a 'apr_size_t' ?
> > >
> > > This seems to be the only warning generated from the Win2k build
> > >
> > > http_protocol.c
> > > c:\src\apache\httpd-2.0\modules\http\http_protocol.c(678) : warning
> > > C4244: 'function' : conversion from '__int64 ' to 'unsigned int ',
> > > possible loss of data
> > >
> > > the offending parameter is 'readbytes'
> > >
> > > apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
> > > ap_input_mode_t mode, apr_off_t *readbytes)
> >
> > As of late, the rule has become this: individual buckets can contain no
> > more than an apr_size_t.  A brigade can have total length up to an
> > apr_off_t.  So apr_bucket_split() and friends SHOULD take an apr_size_t,
> > because that's the biggest the bucket can be anyway.  Something within
> > ap_http_filter needs to handle the conversion, I'd expect.
>
> FILE, PIPE, and SOCKET buckets can easily be more than "memory-sized".
> Thus, all of the split and partition functions should take an apr_off_t.
>
> When you read, you'll only get back a portion.
>
> So... the "rule" you state ought to be changed. It just doesn't apply to
> certain types of buckets.

Buckets are buckets.  To have different rules for some buckets than others means that the 
API's would be inconsistant.  Plus, PIPE and SOCKET buckets always have a -1 length.
FILE buckets are basically capped at apr_size_t, because having more than that in one
bucket doesn't really help you anyway.  The argument was that every operating system with
sendfile we support will do a partial write of the file, because 2Gig is too big to send at one time.
Without this change, we get all sorts of warnings on platforms where apr_size_t and apr_off_t
aren't the same.

Ryan

_____________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
-----------------------------------------------------------------------------

Re: apr_bucket_split : should it take a offset or a size?

Posted by Greg Stein <gs...@lyra.org>.
On Wed, Aug 08, 2001 at 01:40:33AM -0400, Cliff Woolley wrote:
> On Tue, 7 Aug 2001, Ian Holsman wrote:
> 
> > should the 'point' parameter in the bucket split (shared/notimpl/ & simple)
> > be a 'apr_off_t' or a 'apr_size_t' ?
> >
> > This seems to be the only warning generated from the Win2k build
> >
> > http_protocol.c
> > c:\src\apache\httpd-2.0\modules\http\http_protocol.c(678) : warning
> > C4244: 'function' : conversion from '__int64 ' to 'unsigned int ',
> > possible loss of data
> >
> > the offending parameter is 'readbytes'
> >
> > apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
> > ap_input_mode_t mode, apr_off_t *readbytes)
> 
> As of late, the rule has become this: individual buckets can contain no
> more than an apr_size_t.  A brigade can have total length up to an
> apr_off_t.  So apr_bucket_split() and friends SHOULD take an apr_size_t,
> because that's the biggest the bucket can be anyway.  Something within
> ap_http_filter needs to handle the conversion, I'd expect.

FILE, PIPE, and SOCKET buckets can easily be more than "memory-sized". Thus,
all of the split and partition functions should take an apr_off_t.

When you read, you'll only get back a portion.

So... the "rule" you state ought to be changed. It just doesn't apply to
certain types of buckets.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: apr_bucket_split : should it take a offset or a size?

Posted by Cliff Woolley <cl...@yahoo.com>.
On Tue, 7 Aug 2001, Ian Holsman wrote:

> should the 'point' parameter in the bucket split (shared/notimpl/ & simple)
> be a 'apr_off_t' or a 'apr_size_t' ?
>
> This seems to be the only warning generated from the Win2k build
>
> http_protocol.c
> c:\src\apache\httpd-2.0\modules\http\http_protocol.c(678) : warning
> C4244: 'function' : conversion from '__int64 ' to 'unsigned int ',
> possible loss of data
>
> the offending parameter is 'readbytes'
>
> apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
> ap_input_mode_t mode, apr_off_t *readbytes)

As of late, the rule has become this: individual buckets can contain no
more than an apr_size_t.  A brigade can have total length up to an
apr_off_t.  So apr_bucket_split() and friends SHOULD take an apr_size_t,
because that's the biggest the bucket can be anyway.  Something within
ap_http_filter needs to handle the conversion, I'd expect.

--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA