svn commit: r587616 - /apr/apr-util/branches/1.2.x/buckets/apr_brigade.c

[wr...@apache.org]

Author: wrowe
Date: Tue Oct 23 12:07:15 2007
New Revision: 587616

URL: http://svn.apache.org/viewvc?rev=587616&view=rev
Log:
Again, appreciating extra scrutiny here.

I beleive my analysis is correct that we protect from
the case where the off_t 'point' arg (signed) falls
out of the scope of size_t e->length (equivilant or
smaller than an off_t, and unsigned).  I don't think
we have to worry about where size_t is larger than off_t,
I believe that logic would even be correct here.

Backports: r587434

Modified:
    apr/apr-util/branches/1.2.x/buckets/apr_brigade.c

Modified: apr/apr-util/branches/1.2.x/buckets/apr_brigade.c
URL: http://svn.apache.org/viewvc/apr/apr-util/branches/1.2.x/buckets/apr_brigade.c?rev=587616&r1=587615&r2=587616&view=diff
==============================================================================
--- apr/apr-util/branches/1.2.x/buckets/apr_brigade.c (original)
+++ apr/apr-util/branches/1.2.x/buckets/apr_brigade.c Tue Oct 23 12:07:15 2007
@@ -114,6 +114,11 @@
          e != APR_BRIGADE_SENTINEL(b);
          e = APR_BUCKET_NEXT(e))
     {
+        /* XXX: 2's compliment math error here, (apr_size_t)(-1) is not
+         * a sufficient replacement for MAX_SIZE_T (compared to 'point' here);
+         * For an unknown length bucket, while 'point' is beyond the possible
+         * size contained in apr_size_t, read and continue...
+         */
         if ((e->length == (apr_size_t)(-1)) && (point > (apr_size_t)(-1))) {
             /* point is too far out to simply split this bucket,
              * we must fix this bucket's size and keep going... */
@@ -123,9 +128,12 @@
                 return rv;
             }
         }
-        if ((point < e->length) || (e->length == (apr_size_t)(-1))) {
-            /* We already checked e->length -1 above, so we now
-             * trust e->length < MAX_APR_SIZE_T.
+        else if (((apr_size_t)point < e->length) || (e->length == (apr_size_t)(-1))) {
+            /* We already consumed buckets where point is beyond 
+             * our interest ( point > MAX_APR_SIZE_T ), above.
+             * Here point falls between 0 and MAX_APR_SIZE_T 
+             * and is within this bucket, or this bucket's len
+             * is undefined, so now we are ready to split it.
              * First try to split the bucket natively... */
             if ((rv = apr_bucket_split(e, (apr_size_t)point)) 
                     != APR_ENOTIMPL) {
@@ -144,7 +152,7 @@
             /* this assumes that len == e->length, which is okay because e
              * might have been morphed by the apr_bucket_read() above, but
              * if it was, the length would have been adjusted appropriately */
-            if (point < e->length) {
+            if ((apr_size_t)point < e->length) {
                 rv = apr_bucket_split(e, (apr_size_t)point);
                 *after_point = APR_BUCKET_NEXT(e);
                 return rv;
@@ -342,7 +350,7 @@
         ++vec;
     }
 
-    *nvec = vec - orig;
+    *nvec = (int)(vec - orig);
     return APR_SUCCESS;
 }