You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stefan Sperling <st...@apache.org> on 2018/12/16 12:28:14 UTC

[PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

mod_deflates hard-codes some off_t format directives to "%ld".
It seems to me this code should use the macro provided by APR instead.

Looking for another pair of eyes. Does this patch look good to commit?

Index: modules/filters/mod_deflate.c
===================================================================
--- modules/filters/mod_deflate.c	(revision 1849023)
+++ modules/filters/mod_deflate.c	(working copy)
@@ -893,7 +893,8 @@ static apr_status_t deflate_out_filter(ap_filter_t
                                        f->c->bucket_alloc);
             APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01384)
-                          "Zlib: Compressed %ld to %ld : URL %s",
+                          "Zlib: Compressed %" APR_OFF_T_FMT
+			  " to %" APR_OFF_T_FMT " : URL %s",
                           ctx->stream.total_in, ctx->stream.total_out, r->uri);
 
             /* leave notes for logging */
@@ -1438,7 +1439,8 @@ static apr_status_t deflate_in_filter(ap_filter_t
                 ctx->validation_buffer_length += valid;
 
                 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01393)
-                              "Zlib: Inflated %ld to %ld : URL %s",
+                              "Zlib: Inflated %" APR_OFF_T_FMT
+			      " to %" APR_OFF_T_FMT " : URL %s",
                               ctx->stream.total_in, ctx->stream.total_out,
                               r->uri);
 
@@ -1459,7 +1461,8 @@ static apr_status_t deflate_in_filter(ap_filter_t
                     if ((ctx->stream.total_out & 0xFFFFFFFF) != compLen) {
                         inflateEnd(&ctx->stream);
                         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01395)
-                                      "Zlib: Length %ld of inflated data does "
+                                      "Zlib: Length %" APR_OFF_T_FMT
+				      " of inflated data does "
                                       "not match expected value %ld",
                                       ctx->stream.total_out, compLen);
                         return APR_EGENERAL;
@@ -1628,7 +1631,8 @@ static apr_status_t inflate_out_filter(ap_filter_t
              */
             flush_libz_buffer(ctx, c, inflate, Z_SYNC_FLUSH, UPDATE_CRC);
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01398)
-                          "Zlib: Inflated %ld to %ld : URL %s",
+                          "Zlib: Inflated %" APR_OFF_T_FMT
+			  " to %" APR_OFF_T_FMT " : URL %s",
                           ctx->stream.total_in, ctx->stream.total_out, r->uri);
 
             if (ctx->validation_buffer_length == VALIDATION_SIZE) {

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Stefan Sperling <st...@apache.org>.
On Sun, Dec 23, 2018 at 02:32:30PM +0100, Yann Ylavic wrote:
> Thanks Stefan, I didn't notice before in your proposed patch, but it
> looks like uint64_t casts should be apr_uint64_t too.
> 
> Regards,
> Yann.

Right. I went ahead and fixed it in r1849630.

Thanks,
Stefan

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Dec 23, 2018 at 10:33 AM Stefan Sperling <st...@apache.org> wrote:
>
> On Wed, Dec 19, 2018 at 07:03:39PM +0100, Stefan Sperling wrote:
> > On Wed, Dec 19, 2018 at 02:58:28PM +0100, Yann Ylavic wrote:
> > > On Wed, Dec 19, 2018 at 9:53 AM Stefan Sperling <st...@apache.org> wrote:
> > > >
> > > > On Tue, Dec 18, 2018 at 12:29:18AM +0100, Yann Ylavic wrote:
> > > > > But yes, upcast is better, while at it I'd go for uint64_t...
> > > >
> > > > Like this?
> > >
> > > I think APR_UINT64_T_FMT/apr_uint64_t would be more portable ;)
> > >
> > > Thanks for taking care of this !
> >
> > Sure :)
>
> Committed.

Thanks Stefan, I didn't notice before in your proposed patch, but it
looks like uint64_t casts should be apr_uint64_t too.

Regards,
Yann.

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Stefan Sperling <st...@apache.org>.
On Wed, Dec 19, 2018 at 07:03:39PM +0100, Stefan Sperling wrote:
> On Wed, Dec 19, 2018 at 02:58:28PM +0100, Yann Ylavic wrote:
> > On Wed, Dec 19, 2018 at 9:53 AM Stefan Sperling <st...@apache.org> wrote:
> > >
> > > On Tue, Dec 18, 2018 at 12:29:18AM +0100, Yann Ylavic wrote:
> > > > But yes, upcast is better, while at it I'd go for uint64_t...
> > >
> > > Like this?
> > 
> > I think APR_UINT64_T_FMT/apr_uint64_t would be more portable ;)
> > 
> > Thanks for taking care of this !
> 
> Sure :)

Committed.
 
> Index: modules/filters/mod_deflate.c
> ===================================================================
> --- modules/filters/mod_deflate.c	(revision 1849274)
> +++ modules/filters/mod_deflate.c	(working copy)
> @@ -893,8 +893,10 @@ static apr_status_t deflate_out_filter(ap_filter_t
>                                         f->c->bucket_alloc);
>              APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
>              ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01384)
> -                          "Zlib: Compressed %ld to %ld : URL %s",
> -                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
> +                          "Zlib: Compressed %" APR_UINT64_T_FMT
> +                          " to %" APR_UINT64_T_FMT " : URL %s",
> +                          (uint64_t)ctx->stream.total_in,
> +                          (uint64_t)ctx->stream.total_out, r->uri);
>  
>              /* leave notes for logging */
>              if (c->note_input_name) {
> @@ -1438,9 +1440,10 @@ static apr_status_t deflate_in_filter(ap_filter_t
>                  ctx->validation_buffer_length += valid;
>  
>                  ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01393)
> -                              "Zlib: Inflated %ld to %ld : URL %s",
> -                              ctx->stream.total_in, ctx->stream.total_out,
> -                              r->uri);
> +                              "Zlib: Inflated %" APR_UINT64_T_FMT
> +                              " to %" APR_UINT64_T_FMT " : URL %s",
> +                              (uint64_t)ctx->stream.total_in,
> +                              (uint64_t)ctx->stream.total_out, r->uri);
>  
>                  consume_buffer(ctx, c, c->bufferSize - ctx->stream.avail_out,
>                                 UPDATE_CRC, ctx->proc_bb);
> @@ -1459,9 +1462,10 @@ static apr_status_t deflate_in_filter(ap_filter_t
>                      if ((ctx->stream.total_out & 0xFFFFFFFF) != compLen) {
>                          inflateEnd(&ctx->stream);
>                          ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01395)
> -                                      "Zlib: Length %ld of inflated data does "
> -                                      "not match expected value %ld",
> -                                      ctx->stream.total_out, compLen);
> +                                      "Zlib: Length %" APR_UINT64_T_FMT
> +                                      " of inflated data does not match"
> +                                      " expected value %ld",
> +                                      (uint64_t)ctx->stream.total_out, compLen);
>                          return APR_EGENERAL;
>                      }
>                  }
> @@ -1628,8 +1632,10 @@ static apr_status_t inflate_out_filter(ap_filter_t
>               */
>              flush_libz_buffer(ctx, c, inflate, Z_SYNC_FLUSH, UPDATE_CRC);
>              ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01398)
> -                          "Zlib: Inflated %ld to %ld : URL %s",
> -                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
> +                          "Zlib: Inflated %" APR_UINT64_T_FMT 
> +                          " to %" APR_UINT64_T_FMT " : URL %s",
> +                          (uint64_t)ctx->stream.total_in,
> +                          (uint64_t)ctx->stream.total_out, r->uri);
>  
>              if (ctx->validation_buffer_length == VALIDATION_SIZE) {
>                  unsigned long compCRC, compLen;

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Stefan Sperling <st...@apache.org>.
On Wed, Dec 19, 2018 at 02:58:28PM +0100, Yann Ylavic wrote:
> On Wed, Dec 19, 2018 at 9:53 AM Stefan Sperling <st...@apache.org> wrote:
> >
> > On Tue, Dec 18, 2018 at 12:29:18AM +0100, Yann Ylavic wrote:
> > > But yes, upcast is better, while at it I'd go for uint64_t...
> >
> > Like this?
> 
> I think APR_UINT64_T_FMT/apr_uint64_t would be more portable ;)
> 
> Thanks for taking care of this !

Sure :)

Index: modules/filters/mod_deflate.c
===================================================================
--- modules/filters/mod_deflate.c	(revision 1849274)
+++ modules/filters/mod_deflate.c	(working copy)
@@ -893,8 +893,10 @@ static apr_status_t deflate_out_filter(ap_filter_t
                                        f->c->bucket_alloc);
             APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01384)
-                          "Zlib: Compressed %ld to %ld : URL %s",
-                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
+                          "Zlib: Compressed %" APR_UINT64_T_FMT
+                          " to %" APR_UINT64_T_FMT " : URL %s",
+                          (uint64_t)ctx->stream.total_in,
+                          (uint64_t)ctx->stream.total_out, r->uri);
 
             /* leave notes for logging */
             if (c->note_input_name) {
@@ -1438,9 +1440,10 @@ static apr_status_t deflate_in_filter(ap_filter_t
                 ctx->validation_buffer_length += valid;
 
                 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01393)
-                              "Zlib: Inflated %ld to %ld : URL %s",
-                              ctx->stream.total_in, ctx->stream.total_out,
-                              r->uri);
+                              "Zlib: Inflated %" APR_UINT64_T_FMT
+                              " to %" APR_UINT64_T_FMT " : URL %s",
+                              (uint64_t)ctx->stream.total_in,
+                              (uint64_t)ctx->stream.total_out, r->uri);
 
                 consume_buffer(ctx, c, c->bufferSize - ctx->stream.avail_out,
                                UPDATE_CRC, ctx->proc_bb);
@@ -1459,9 +1462,10 @@ static apr_status_t deflate_in_filter(ap_filter_t
                     if ((ctx->stream.total_out & 0xFFFFFFFF) != compLen) {
                         inflateEnd(&ctx->stream);
                         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01395)
-                                      "Zlib: Length %ld of inflated data does "
-                                      "not match expected value %ld",
-                                      ctx->stream.total_out, compLen);
+                                      "Zlib: Length %" APR_UINT64_T_FMT
+                                      " of inflated data does not match"
+                                      " expected value %ld",
+                                      (uint64_t)ctx->stream.total_out, compLen);
                         return APR_EGENERAL;
                     }
                 }
@@ -1628,8 +1632,10 @@ static apr_status_t inflate_out_filter(ap_filter_t
              */
             flush_libz_buffer(ctx, c, inflate, Z_SYNC_FLUSH, UPDATE_CRC);
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01398)
-                          "Zlib: Inflated %ld to %ld : URL %s",
-                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
+                          "Zlib: Inflated %" APR_UINT64_T_FMT 
+                          " to %" APR_UINT64_T_FMT " : URL %s",
+                          (uint64_t)ctx->stream.total_in,
+                          (uint64_t)ctx->stream.total_out, r->uri);
 
             if (ctx->validation_buffer_length == VALIDATION_SIZE) {
                 unsigned long compCRC, compLen;

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Dec 19, 2018 at 9:53 AM Stefan Sperling <st...@apache.org> wrote:
>
> On Tue, Dec 18, 2018 at 12:29:18AM +0100, Yann Ylavic wrote:
> > But yes, upcast is better, while at it I'd go for uint64_t...
>
> Like this?

I think APR_UINT64_T_FMT/apr_uint64_t would be more portable ;)

Thanks for taking care of this !

Regards,
Yann.

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Stefan Sperling <st...@apache.org>.
On Tue, Dec 18, 2018 at 12:29:18AM +0100, Yann Ylavic wrote:
> But yes, upcast is better, while at it I'd go for uint64_t...

Like this?

I've noticed that the same problem seems to exist in some other modules.
I'll send separate patches for those once this patch has settled.

Index: modules/filters/mod_deflate.c
===================================================================
--- modules/filters/mod_deflate.c	(revision 1849274)
+++ modules/filters/mod_deflate.c	(working copy)
@@ -893,8 +893,9 @@ static apr_status_t deflate_out_filter(ap_filter_t
                                        f->c->bucket_alloc);
             APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01384)
-                          "Zlib: Compressed %ld to %ld : URL %s",
-                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
+                          "Zlib: Compressed %llu to %llu: URL %s",
+                          (uint64_t)ctx->stream.total_in,
+                          (uint64_t)ctx->stream.total_out, r->uri);
 
             /* leave notes for logging */
             if (c->note_input_name) {
@@ -1438,9 +1439,9 @@ static apr_status_t deflate_in_filter(ap_filter_t
                 ctx->validation_buffer_length += valid;
 
                 ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01393)
-                              "Zlib: Inflated %ld to %ld : URL %s",
-                              ctx->stream.total_in, ctx->stream.total_out,
-                              r->uri);
+                              "Zlib: Inflated %llu to %llu : URL %s",
+                              (uint64_t)ctx->stream.total_in,
+                              (uint64_t)ctx->stream.total_out, r->uri);
 
                 consume_buffer(ctx, c, c->bufferSize - ctx->stream.avail_out,
                                UPDATE_CRC, ctx->proc_bb);
@@ -1459,9 +1460,9 @@ static apr_status_t deflate_in_filter(ap_filter_t
                     if ((ctx->stream.total_out & 0xFFFFFFFF) != compLen) {
                         inflateEnd(&ctx->stream);
                         ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01395)
-                                      "Zlib: Length %ld of inflated data does "
+                                      "Zlib: Length %llu of inflated data does "
                                       "not match expected value %ld",
-                                      ctx->stream.total_out, compLen);
+                                      (uint64_t)ctx->stream.total_out, compLen);
                         return APR_EGENERAL;
                     }
                 }
@@ -1628,8 +1629,9 @@ static apr_status_t inflate_out_filter(ap_filter_t
              */
             flush_libz_buffer(ctx, c, inflate, Z_SYNC_FLUSH, UPDATE_CRC);
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(01398)
-                          "Zlib: Inflated %ld to %ld : URL %s",
-                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
+                          "Zlib: Inflated %llu to %llu: URL %s",
+                          (uint64_t)ctx->stream.total_in,
+                          (uint64_t)ctx->stream.total_out, r->uri);
 
             if (ctx->validation_buffer_length == VALIDATION_SIZE) {
                 unsigned long compCRC, compLen;

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Yann Ylavic <yl...@gmail.com>.
On Mon, Dec 17, 2018 at 5:40 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> On Sun, Dec 16, 2018 at 7:27 AM Yann Ylavic <yl...@gmail.com> wrote:
>>
>>
>> Since it's logging only, it may be easier to cast to (long) each
>> total_in/out though.
>
> Downcast? Why not upcast to apr_off_t and use the _FMT macro as first suggested?

Neither is ideal I think since in the unsigned long case that's still
a cast to signed off_t.
But yes, upcast is better, while at it I'd go for uint64_t...

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Sun, Dec 16, 2018 at 7:27 AM Yann Ylavic <yl...@gmail.com> wrote:

>
> Since it's logging only, it may be easier to cast to (long) each
> total_in/out though.
>

Downcast? Why not upcast to apr_off_t and use the _FMT macro as first
suggested?

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Dec 16, 2018 at 2:21 PM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Sun, Dec 16, 2018 at 2:14 PM Stefan Sperling <st...@apache.org> wrote:
> >
> > The file /usr/include/zlib.h I have on OpenBSD -current has this:
> >
> > typedef struct z_stream_s {
> >     [...]
> >     z_off_t  total_in;  /* total nb of input bytes read so far */
> >     [...]
> >     z_off_t  total_out; /* total nb of bytes output so far */
> >     [...]
> > } z_stream;
>
> Ouch.
>
> >
> > This looks like an OpenBSD-specific change though (diff below).
> > I guess this will force OpenBSD to carry a local patch for
> > mod_deflate then, or just compile without -Werror.
>
> I think we need a bit of #ifdef-ery in mod_deflate then, a BSD only
> fix wouldn't help those who build from sources.
>
> Do you know which BSD(s) are concerned (if not all)?

Since it's logging only, it may be easier to cast to (long) each
total_in/out though.

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Dec 16, 2018 at 2:14 PM Stefan Sperling <st...@apache.org> wrote:
>
> The file /usr/include/zlib.h I have on OpenBSD -current has this:
>
> typedef struct z_stream_s {
>     [...]
>     z_off_t  total_in;  /* total nb of input bytes read so far */
>     [...]
>     z_off_t  total_out; /* total nb of bytes output so far */
>     [...]
> } z_stream;

Ouch.

>
> This looks like an OpenBSD-specific change though (diff below).
> I guess this will force OpenBSD to carry a local patch for
> mod_deflate then, or just compile without -Werror.

I think we need a bit of #ifdef-ery in mod_deflate then, a BSD only
fix wouldn't help those who build from sources.

Do you know which BSD(s) are concerned (if not all)?

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Stefan Sperling <st...@apache.org>.
On Sun, Dec 16, 2018 at 02:03:45PM +0100, Yann Ylavic wrote:
> On Sun, Dec 16, 2018 at 1:28 PM Stefan Sperling <st...@apache.org> wrote:
> >
> > mod_deflates hard-codes some off_t format directives to "%ld".
> > It seems to me this code should use the macro provided by APR instead.
> >
> > Looking for another pair of eyes. Does this patch look good to commit?
> 
> It seems that zlib defines total_in/out as uLong, i.e.:
> typedef struct z_stream_s {
>     [...]
>     uLong    total_in;  /* total number of input bytes read so far */
>     [...]
>     uLong    total_out; /* total number of bytes output so far */
>     [...]
> } z_stream;
> 
> So %ld (or %lu) looks correct to me.
> 
> (mod_brotli uses apr_off_t for them but it's an internal struct there).
> 
> Regards,
> Yann.

Thanks for checking. This seems to depend on the version of zlib.
The file /usr/include/zlib.h I have on OpenBSD -current has this:

typedef struct z_stream_s {
    [...]
    z_off_t  total_in;  /* total nb of input bytes read so far */
    [...]
    z_off_t  total_out; /* total nb of bytes output so far */
    [...]
} z_stream;

And the compiler (clang) now rightly complains as follows:

/home/stsp/svn/src/httpd-2.4.37/modules/filters/mod_deflate.c:856:27: error:   
      format specifies type 'long' but the argument has type 'off_t'           
      (aka 'long long') [-Werror,-Wformat]                                     
                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
                          ^~~~~~~~~~~~~~~~~~~~                                 
/home/stsp/svn/src/httpd-2.4.37/modules/filters/mod_deflate.c:856:49: error:   
      format specifies type 'long' but the argument has type 'off_t'           
      (aka 'long long') [-Werror,-Wformat]                                     
                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
                                                ^~~~~~~~~~~~~~~~~~~~~          
/home/stsp/svn/src/httpd-2.4.37/modules/filters/mod_deflate.c:1423:31: error:  
      format specifies type 'long' but the argument has type 'off_t'           
      (aka 'long long') [-Werror,-Wformat]                                     
                              ctx->stream.total_in, ctx->stream.total_out,     
                              ^~~~~~~~~~~~~~~~~~~~                             
/home/stsp/svn/src/httpd-2.4.37/modules/filters/mod_deflate.c:1423:53: error:  
      format specifies type 'long' but the argument has type 'off_t'           
      (aka 'long long') [-Werror,-Wformat]                                     
                              ctx->stream.total_in, ctx->stream.total_out,     
                                                    ^~~~~~~~~~~~~~~~~~~~~      
/home/stsp/svn/src/httpd-2.4.37/modules/filters/mod_deflate.c:1450:39: error:  
      format specifies type 'long' but the argument has type 'off_t'           
      (aka 'long long') [-Werror,-Wformat]                                     
                                      ctx->stream.total_out, compLen);         
                                      ^~~~~~~~~~~~~~~~~~~~~                    
/home/stsp/svn/src/httpd-2.4.37/modules/filters/mod_deflate.c:1626:27: error:  
      format specifies type 'long' but the argument has type 'off_t'           
      (aka 'long long') [-Werror,-Wformat]                                     
                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
                          ^~~~~~~~~~~~~~~~~~~~                                 
/home/stsp/svn/src/httpd-2.4.37/modules/filters/mod_deflate.c:1626:49: error:  
      format specifies type 'long' but the argument has type 'off_t'           
      (aka 'long long') [-Werror,-Wformat]                                     
                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
                                                ^~~~~~~~~~~~~~~~~~~~~          
7 errors generated.

This looks like an OpenBSD-specific change though (diff below).
I guess this will force OpenBSD to carry a local patch for
mod_deflate then, or just compile without -Werror.

RCS file: /cvs/src/lib/libz/zlib.h,v
Working file: zlib.h
head: 1.10
    [...]
----------------------------
revision 1.7
date: 2003/12/16 22:35:50;  author: henning;  state: Exp;  lines: +2 -2;
total_in and total_out need to be off_t, not unsigned long.
some bugs return: i fixed the same some months ago when we had this other gzip
there.
this bug resulted in wrong size stats for > 4GB files, and in the case
that the input file was > 4GB and could be compressed to < 4GB gzip
not zipping it as it would grow in its eyes.
=============================================================================

Index: zlib.h
===================================================================
RCS file: /cvs/src/lib/libz/zlib.h,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -p -r1.6 -r1.7
--- zlib.h	16 Dec 2003 22:33:02 -0000	1.6
+++ zlib.h	16 Dec 2003 22:35:50 -0000	1.7
@@ -1,4 +1,4 @@
-/*	$OpenBSD: zlib.h,v 1.6 2003/12/16 22:33:02 henning Exp $	*/
+/*	$OpenBSD: zlib.h,v 1.7 2003/12/16 22:35:50 henning Exp $	*/
 /* zlib.h -- interface of the 'zlib' general purpose compression library
   version 1.2.1, November 17th, 2003
 
@@ -85,11 +85,11 @@ struct internal_state;
 typedef struct z_stream_s {
     Bytef    *next_in;  /* next input byte */
     uInt     avail_in;  /* number of bytes available at next_in */
-    uLong    total_in;  /* total nb of input bytes read so far */
+    z_off_t  total_in;  /* total nb of input bytes read so far */
 
     Bytef    *next_out; /* next output byte should be put there */
     uInt     avail_out; /* remaining free space at next_out */
-    uLong    total_out; /* total nb of bytes output so far */
+    z_off_t  total_out; /* total nb of bytes output so far */
 
     char     *msg;      /* last error message, NULL if no error */
     struct internal_state FAR *state; /* not visible by applications */

Re: [PATCH] mod_deflate: hardcoded "%ld" -> APR_OFF_T_FMT

Posted by Yann Ylavic <yl...@gmail.com>.
On Sun, Dec 16, 2018 at 1:28 PM Stefan Sperling <st...@apache.org> wrote:
>
> mod_deflates hard-codes some off_t format directives to "%ld".
> It seems to me this code should use the macro provided by APR instead.
>
> Looking for another pair of eyes. Does this patch look good to commit?

It seems that zlib defines total_in/out as uLong, i.e.:
typedef struct z_stream_s {
    [...]
    uLong    total_in;  /* total number of input bytes read so far */
    [...]
    uLong    total_out; /* total number of bytes output so far */
    [...]
} z_stream;

So %ld (or %lu) looks correct to me.

(mod_brotli uses apr_off_t for them but it's an internal struct there).

Regards,
Yann.