You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by rb...@covalent.net on 2000/06/28 23:45:52 UTC

[PATCH] potential fix for the APR_OFF_T_FMT warning

I am submitting this patch, because I don't know if it is a good idea or
not.  Part of me likes it, part of me thinks it's a BIG hack.  

Basically, we just swap the order in which we check the ap_off_t size.  It
sounds far too easy to work, but I think it might be good enough.

Basically, my thinking is that this is a gcc-only thing, and it is
potentially a gcc-on-linux-only thing.  Although I doubt that.

I am taking chances with this patch.  I am assuming that if a platform has
int and long int that are both the same length, then the developers chose
long int for off_t because it will make things easier in the future.  This
may be a HUGE gamble.

Please, post comments, tell me I'm crazy, anything.  If people don't like
this, I'll try to fix the problem another way.

Ryan

Index: lib/apr/configure.in
===================================================================
RCS file: /home/cvs/apache-2.0/src/lib/apr/configure.in,v
retrieving revision 1.128
diff -u -d -b -w -u -r1.128 configure.in
--- lib/apr/configure.in	2000/06/28 16:31:07	1.128
+++ lib/apr/configure.in	2000/06/28 21:39:53
@@ -370,12 +370,12 @@
 
 AC_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], off_t, 8)
 
-if test "$ac_cv_sizeof_off_t" = "$ac_cv_sizeof_int"; then
-    off_t_fmt='#define APR_OFF_T_FMT "d"'
+if test "$ac_cv_sizeof_off_t" = "$ac_cv_sizeof_long_long"; then
+    off_t_fmt='#define APR_OFF_T_FMT "qd"'
 elif test "$ac_cv_sizeof_off_t" = "$ac_cv_sizeof_long"; then
     off_t_fmt='#define APR_OFF_T_FMT "ld"'
-elif test "$ac_cv_sizeof_off_t" = "$ac_cv_sizeof_long_long"; then
-    off_t_fmt='#define APR_OFF_T_FMT "qd"'
+elif test "$ac_cv_sizeof_off_t" = "$ac_cv_sizeof_int"; then
+    off_t_fmt='#define APR_OFF_T_FMT "d"'
 else
     off_t_fmt='#error Can not determine the proper size for off_t'
 fi

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: [PATCH] potential fix for the APR_OFF_T_FMT warning

Posted by Tony Finch <do...@dotat.at>.
From: Jeff Trawick <tr...@ibm.net> wrote:
>
>On FreeBSD, off_t is int whereas on Linux it is long.

No, it's 64 bits on all BSDs, i.e. long long or equivalent -- for some
reason with gcc on FreeBSD on i386 it's
	typedef int __attribute__((__mode__(__DI__))) __int64_t;
(possibly to make it work in ANSI BDSM mode)

>Frankly, I think your earlier suggesting about hardcoding it per
>platform may part of the clearest solution.

I expect the patch will work on most platforms, except perhaps 64 bit
platforms where these types are declared as long instead of long long
:-)

>By the way...   Add AP_SSIZE_T_FMT while you're at it :)

ssize_t is more usually 32 bits so I'd expect the analogous patch to
work even better for it.

Tony.
-- 
f.a.n.finch    fanf@covalent.net    dot@dotat.at
446 backflip from the balcony

Re: [PATCH] potential fix for the APR_OFF_T_FMT warning

Posted by rb...@covalent.net.
> > Frankly, I think your earlier suggesting about hardcoding it per
> > platform may part of the clearest solution.
> 
> This seems like the only way it'll work cleanly across a large number of
> platforms.

I'm not convinced of that yet.  Let me try one more thing.  I was trying
to avoid this next patch, but it looks like the only way to solve this
without hard-coding.  :-(

> > By the way...   Add AP_SSIZE_T_FMT while you're at it :)
> 
> There may be others for different platforms as well :(

The only two types we really need to worry about for providing formats are
ap_ssize_t and ap_off_t.  The same solution will work for both, we just
need to find it.  :-)  Patch later today.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: [PATCH] potential fix for the APR_OFF_T_FMT warning

Posted by David Reid <dr...@jetnet.co.uk>.
> Unless I'm confused, your patch won't change this; it will just move
> the warning from Linux to FreeBSD.

Seems likely.

>
> Frankly, I think your earlier suggesting about hardcoding it per
> platform may part of the clearest solution.

This seems like the only way it'll work cleanly across a large number of
platforms.

>
> Suppose that with your current patch the order of checking helps us
> determine APR_OFF_T_FMT correctly for most systems.  Use your patch
> as-is, but after that code do this to reset off_t_fmt for cases where
> the automatic check fails:
>
> case "$PLAT" in
>     *-freebsd*)
>         off_t_fmt='#define APR_OFF_T_FMT "d"'
>     ;;
> esac
>
> By the way...   Add AP_SSIZE_T_FMT while you're at it :)

There may be others for different platforms as well :(

david



Re: [PATCH] potential fix for the APR_OFF_T_FMT warning

Posted by rb...@covalent.net.

> This program generates a warning on FreeBSD but no warning on Linux:

That's what I needed to know.  Okay, I'll hack something together today to
fix all of this.  ARGH!!!!!

Great fun.

Ryan

_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


Re: [PATCH] potential fix for the APR_OFF_T_FMT warning

Posted by Jeff Trawick <tr...@ibm.net>.
> From: rbb@covalent.net
> Date: Wed, 28 Jun 2000 14:45:52 -0700 (PDT)
> 
> I am submitting this patch, because I don't know if it is a good idea or
> not.  Part of me likes it, part of me thinks it's a BIG hack.  
> 
> Basically, we just swap the order in which we check the ap_off_t size.  It
> sounds far too easy to work, but I think it might be good enough.
> 
> Basically, my thinking is that this is a gcc-only thing, and it is
> potentially a gcc-on-linux-only thing.  Although I doubt that.
> 
> I am taking chances with this patch.  I am assuming that if a platform has
> int and long int that are both the same length, then the developers chose
> long int for off_t because it will make things easier in the future.  This
> may be a HUGE gamble.

On FreeBSD, off_t is int whereas on Linux it is long.

This program generates a warning on FreeBSD but no warning on Linux:

#include <stdio.h>
#include <sys/types.h>
 
int main(void)
{
  off_t value = 0;
 
  printf("%ld",value);
  return 0;
}     

trawick@freebie.local:/usr/home/trawick/apache/apache-2.0/src% gcc -Wall tst.c
tst.c: In function `main':
tst.c:9: warning: long int format, different type arg (arg 2) 

Unless I'm confused, your patch won't change this; it will just move
the warning from Linux to FreeBSD.

Frankly, I think your earlier suggesting about hardcoding it per
platform may part of the clearest solution.

Suppose that with your current patch the order of checking helps us
determine APR_OFF_T_FMT correctly for most systems.  Use your patch
as-is, but after that code do this to reset off_t_fmt for cases where
the automatic check fails:

case "$PLAT" in
    *-freebsd*)
        off_t_fmt='#define APR_OFF_T_FMT "d"'
    ;;
esac

By the way...   Add AP_SSIZE_T_FMT while you're at it :)

Have fun,

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...