osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


On Sun, Dec 16, 2018 at 02:03:45PM +0100, Yann Ylavic wrote:
> On Sun, Dec 16, 2018 at 1:28 PM Stefan Sperling <stsp@xxxxxxxxxx> 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 */