You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jo...@apache.org on 2007/10/29 12:19:03 UTC
svn commit: r589583 - /apr/apr/trunk/configure.in
Author: jorton
Date: Mon Oct 29 04:19:02 2007
New Revision: 589583
URL: http://svn.apache.org/viewvc?rev=589583&view=rev
Log:
* configure.in: Move the ino_t test outside the enable_lfs=yes branch,
and only define apr_ino_t as unsigned long in the case where a 32-bit
ino_t is detected and hence may vary by _FILE_OFFSET_BITS; it's not
necessary in the other cases.
Modified:
apr/apr/trunk/configure.in
Modified: apr/apr/trunk/configure.in
URL: http://svn.apache.org/viewvc/apr/apr/trunk/configure.in?rev=589583&r1=589582&r2=589583&view=diff
==============================================================================
--- apr/apr/trunk/configure.in (original)
+++ apr/apr/trunk/configure.in Mon Oct 29 04:19:02 2007
@@ -530,14 +530,6 @@
if test "$apr_cv_use_lfs64" = "yes"; then
APR_ADDTO(CPPFLAGS, [-D_LARGEFILE64_SOURCE])
fi
-
-
- dnl define apr_ino_t in a manner independent of _FILE_OFFSET_BITS setting
- dnl default fallback
- ino_t_value=ino_t
- APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long long, ino_t_value="unsigned long long")
- APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long, ino_t_value="unsigned long")
- APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned int, ino_t_value="unsigned int")
fi
AC_ARG_ENABLE(nonportable-atomics,
@@ -1462,6 +1454,24 @@
off_t_strfn='strtoi'
fi
AC_MSG_RESULT($off_t_value)
+
+# Regardless of whether _LARGEFILE64_SOURCE is used, on 32-bit
+# platforms _FILE_OFFSET_BITS will affect the size of ino_t and hence
+# the build-time ABI may be different from the apparent ABI when using
+# APR with another package which *does* define _FILE_OFFSET_BITS.
+# (Exactly as per the case above with off_t where LFS is *not* used)
+#
+# To be safe, hard-code apr_ino_t as 'unsigned long' iff that is
+# exactly the size of ino_t here; otherwise use ino_t as existing
+# releases did. To be correct, apr_ino_t should have been made an
+# ino64_t as apr_off_t is off64_t, but this can't be done now without
+# breaking ABI.
+ino_t_value=ino_t
+if test "$ac_cv_sizeof_long" = "4"; then
+ APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long,
+ ino_t_value="unsigned long")
+fi
+AC_MSG_NOTICE([using $ino_t_value for ino_t])
APR_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], pid_t, 8)
Re: svn commit: r589583 - /apr/apr/trunk/configure.in
Posted by Lucian Adrian Grijincu <lu...@gmail.com>.
On 10/29/07, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
> Joe Orton wrote:
> >
> > No, unless you know of a platform where ino_t is defined to be anything
> > other than a 32-bit unsigned long *and* varies by _FILE_OFFSET_BITS?
> > Otherwise configure is just testing for hypothetical platforms, which is
> > a waste of cycles.
>
> You missed my point. Take RandomOS Version 6.0 - build APR, no variant for
> _FILE_OFFSET_BITS, it's equivilant to unsigned int.
>
> Now run an upgrade to RandomOS V7.0, voila, it picks up long off_t semantics
> and 32 more ino_t bits, whoopie!
>
this is the default, so if the next check fails, there can be no ABI
change if there is no variant for _FILE_OFFSET_BITS.
+ino_t_value=ino_t
> Compile an app against the previously installed apr and poof, bang, dead.
>
> Yes, it takes a few extra cycles, but can we /please/ continue to validate
> against unsigned int as well as unsigned long for the sanity of the poor
> users sitting on this edge case? If you wanted to start excluding such
> things, there's a much longer list of tests that need serious surgery or
> outright purging (db detection tops that list).
>
If I understand this correctly:
ino_t can have only two sizes that depend on _FILE_OFFSET_BITS: 32 and 64.
ALL references I could find on LFS talk about an increase from 32 to 64 bits.
Other operating systems may have > 64 bit (I've never heard of one,
but who knows, "2 ^ 64 = 18446744073709551616 files should be enough
for everyone" - you can quote me on that one :) but these ones don't
need LFS, thus no need to replace ino_t with anything else, it won't
change with _FILE_OFFSET_BITS (of course, I base this on nothing
because I've never seen such a system).
I don't know if a 16 bit one exists (65536 files / dirs seems kind of
lowish), this case might use a _FILE_OFFSET_BITS=XY. If such a
platform exists AND uses LFS, then YES, we should check for unsigned
short too and add it to Makefile.in.
The last two cases that remain are 32bit and 64bit:
a) 32 bit. ino_t might change with _FILE_OFFSET_BITS and should be
nailed to an unsigned 32 bit wide integer.
+if test "$ac_cv_sizeof_long" = "4"; then
+ APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long,
+ ino_t_value="unsigned long")
+fi
+AC_MSG_NOTICE([using $ino_t_value for ino_t])
This will fix ino_t to unsigned long if unsigned long is 4 bytes wide,
but will leave it as ino_t if long is more than that (and I see no
reason why long cannot be 8 bytes and while int would be 4 bytes).
I think this satisfies sufficiency:
if test "$ac_cv_sizeof_long" = "4"; then
APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long,
ino_t_value="unsigned long")
fi
if test "$ac_cv_sizeof_int" = "4"; then
APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned int,
ino_t_value="unsigned long")
fi
AC_MSG_NOTICE([using $ino_t_value for ino_t])
Once again, the check for *int* is NECESSARY only on platforms that
are affected by LFS and on which sizeof(long)>sizeof(int). If no such
platform exists Joe's original patch is sufficient.
b) 64 bit wide ino_t. I don't know of a LFS which changes from 64 bit
ino_t to something more. So the default fallback "ino_t_value=ino_t"
is enough.
--
Lucian
Re: svn commit: r589583 - /apr/apr/trunk/configure.in
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> On Mon, Oct 29, 2007 at 02:48:57PM -0500, William Rowe wrote:
>> Joe Orton wrote:
>>> No, unless you know of a platform where ino_t is defined to be anything
>>> other than a 32-bit unsigned long *and* varies by _FILE_OFFSET_BITS?
>>> Otherwise configure is just testing for hypothetical platforms, which is
>>> a waste of cycles.
>> You missed my point. Take RandomOS Version 6.0 - build APR, no variant for
>> _FILE_OFFSET_BITS, it's equivilant to unsigned int.
>>
>> Now run an upgrade to RandomOS V7.0, voila, it picks up long off_t semantics
>> and 32 more ino_t bits, whoopie!
>
> That is not related to the change in question. There are a variety of
> different ways in which APR can potentially change ABI when rebuilt
> after an OS upgrade, but the use of ino_t is not one of them.
Just to be clear, I'm saying - no rebuild of APR. ABI is unchanged. The
consumer of the library is broken. Our apr.h should be (or become) rich
enough to survive such headaches. But with more important issues, I'll
drop this one on the floor rather than belabor the point.
Bill
Re: svn commit: r589583 - /apr/apr/trunk/configure.in
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Oct 29, 2007 at 02:48:57PM -0500, William Rowe wrote:
> Joe Orton wrote:
> >
> >No, unless you know of a platform where ino_t is defined to be anything
> >other than a 32-bit unsigned long *and* varies by _FILE_OFFSET_BITS?
> >Otherwise configure is just testing for hypothetical platforms, which is
> >a waste of cycles.
>
> You missed my point. Take RandomOS Version 6.0 - build APR, no variant for
> _FILE_OFFSET_BITS, it's equivilant to unsigned int.
>
> Now run an upgrade to RandomOS V7.0, voila, it picks up long off_t semantics
> and 32 more ino_t bits, whoopie!
That is not related to the change in question. There are a variety of
different ways in which APR can potentially change ABI when rebuilt
after an OS upgrade, but the use of ino_t is not one of them.
joe
Re: svn commit: r589583 - /apr/apr/trunk/configure.in
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
>
> No, unless you know of a platform where ino_t is defined to be anything
> other than a 32-bit unsigned long *and* varies by _FILE_OFFSET_BITS?
> Otherwise configure is just testing for hypothetical platforms, which is
> a waste of cycles.
You missed my point. Take RandomOS Version 6.0 - build APR, no variant for
_FILE_OFFSET_BITS, it's equivilant to unsigned int.
Now run an upgrade to RandomOS V7.0, voila, it picks up long off_t semantics
and 32 more ino_t bits, whoopie!
Compile an app against the previously installed apr and poof, bang, dead.
Yes, it takes a few extra cycles, but can we /please/ continue to validate
against unsigned int as well as unsigned long for the sanity of the poor
users sitting on this edge case? If you wanted to start excluding such
things, there's a much longer list of tests that need serious surgery or
outright purging (db detection tops that list).
Bill
Re: svn commit: r589583 - /apr/apr/trunk/configure.in
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Oct 29, 2007 at 01:54:24PM -0500, William Rowe wrote:
> jorton@apache.org wrote:
> >Author: jorton
> >Date: Mon Oct 29 04:19:02 2007
> >New Revision: 589583
...
> >+# breaking ABI.
> >+ino_t_value=ino_t
> >+if test "$ac_cv_sizeof_long" = "4"; then
> >+ APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long,
> >+ ino_t_value="unsigned long")
> >+fi
> >+AC_MSG_NOTICE([using $ino_t_value for ino_t])
>
> This doesn't look right for 32 platforms built without 64 bit off's, and
> consider they might initially build on one which doesn't support the newer
> types, but then might compile add ins on an updated kernel, eh?
I can't parse that.
> Shouldn't we continue to lock this /down/ to unsigned int where we know this
> is true? In fact, I'd add unsigned short to that list for safety.
No, unless you know of a platform where ino_t is defined to be anything
other than a 32-bit unsigned long *and* varies by _FILE_OFFSET_BITS?
Otherwise configure is just testing for hypothetical platforms, which is
a waste of cycles.
joe
Re: svn commit: r589583 - /apr/apr/trunk/configure.in
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
jorton@apache.org wrote:
> Author: jorton
> Date: Mon Oct 29 04:19:02 2007
> New Revision: 589583
>
> URL: http://svn.apache.org/viewvc?rev=589583&view=rev
> Log:
> * configure.in: Move the ino_t test outside the enable_lfs=yes branch,
> and only define apr_ino_t as unsigned long in the case where a 32-bit
> ino_t is detected and hence may vary by _FILE_OFFSET_BITS; it's not
> necessary in the other cases.
> @@ -1462,6 +1454,24 @@
> off_t_strfn='strtoi'
> fi
> AC_MSG_RESULT($off_t_value)
> +
> +# Regardless of whether _LARGEFILE64_SOURCE is used, on 32-bit
> +# platforms _FILE_OFFSET_BITS will affect the size of ino_t and hence
> +# the build-time ABI may be different from the apparent ABI when using
> +# APR with another package which *does* define _FILE_OFFSET_BITS.
> +# (Exactly as per the case above with off_t where LFS is *not* used)
> +#
> +# To be safe, hard-code apr_ino_t as 'unsigned long' iff that is
> +# exactly the size of ino_t here; otherwise use ino_t as existing
> +# releases did. To be correct, apr_ino_t should have been made an
> +# ino64_t as apr_off_t is off64_t, but this can't be done now without
> +# breaking ABI.
> +ino_t_value=ino_t
> +if test "$ac_cv_sizeof_long" = "4"; then
> + APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long,
> + ino_t_value="unsigned long")
> +fi
> +AC_MSG_NOTICE([using $ino_t_value for ino_t])
This doesn't look right for 32 platforms built without 64 bit off's, and
consider they might initially build on one which doesn't support the newer
types, but then might compile add ins on an updated kernel, eh?
Shouldn't we continue to lock this /down/ to unsigned int where we know this
is true? In fact, I'd add unsigned short to that list for safety.
Bill