You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Joe Orton <jo...@redhat.com> on 2007/10/12 16:53:26 UTC
Re: ABI change when _FILE_OFFSET_BITS=64 due to use of ino_t
On Tue, Sep 18, 2007 at 08:37:57PM +0300, Lucian Adrian Grijincu wrote:
> I see that some of my messages may have not hit the list:
> http://mail-archives.apache.org/mod_mbox/apr-dev/200709.mbox/browser
> only lists a few.
>
> I've added a bug to bugzilla and attached patches for all three
> development branches: 0.9.x, 1.2.x and trunk.
>
> http://issues.apache.org/bugzilla/show_bug.cgi?id=43417
Thanks for the patches.
The placement of the configure tests was not quite right - it would not
have effect if --disable-lfs was used. I also think it's unnecessary to
hard-code ino_t for anything other than the "unsigned long" case; e.g.
there are no platforms where the native ino_t is a 64-bit type *and*
will vary by _FILE_OFFSET_BITS, to my knowledge.
Here's an updated patch:
Index: configure.in
===================================================================
--- configure.in (revision 583792)
+++ configure.in (working copy)
@@ -1456,6 +1456,21 @@
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
+APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long, ino_t_value="unsigned long")
+AC_MSG_NOTICE([using $ino_t_value for ino_t])
+
APR_CHECK_SIZEOF_EXTENDED([#include <sys/types.h>], pid_t, 8)
if test "$ac_cv_sizeof_pid_t" = "$ac_cv_sizeof_short"; then
@@ -1495,6 +1510,7 @@
AC_SUBST(size_t_value)
AC_SUBST(ssize_t_value)
AC_SUBST(socklen_t_value)
+AC_SUBST(ino_t_value)
AC_SUBST(int64_t_fmt)
AC_SUBST(uint64_t_fmt)
AC_SUBST(uint64_t_hex_fmt)
Index: include/apr.h.in
===================================================================
--- include/apr.h.in (revision 583792)
+++ include/apr.h.in (working copy)
@@ -283,6 +283,7 @@
typedef @ssize_t_value@ apr_ssize_t;
typedef @off_t_value@ apr_off_t;
typedef @socklen_t_value@ apr_socklen_t;
+typedef @ino_t_value@ apr_ino_t;
#define APR_SIZEOF_VOIDP @voidp_size@
Index: include/apr_file_info.h
===================================================================
--- include/apr_file_info.h (revision 583792)
+++ include/apr_file_info.h (working copy)
@@ -125,16 +125,10 @@
typedef apr_int32_t apr_fileperms_t;
#if (defined WIN32) || (defined NETWARE)
/**
- * Structure for determining the inode of the file.
- */
-typedef apr_uint64_t apr_ino_t;
-/**
* Structure for determining the device the file is on.
*/
typedef apr_uint32_t apr_dev_t;
#else
-/** The inode of the file. */
-typedef ino_t apr_ino_t;
/**
* Structure for determining the device the file is on.
*/
Index: include/apr.hw
===================================================================
--- include/apr.hw (revision 583792)
+++ include/apr.hw (working copy)
@@ -346,6 +346,8 @@
#endif
typedef int apr_socklen_t;
+typedef apr_uint64_t apr_ino_t;
+
/* Are we big endian? */
/* XXX: Fatal assumption on Alpha platforms */
#define APR_IS_BIGENDIAN 0
Index: include/apr.hnw
===================================================================
--- include/apr.hnw (revision 583792)
+++ include/apr.hnw (working copy)
@@ -256,6 +256,8 @@
typedef size_t apr_socklen_t;
#endif
+typedef apr_uint64_t apr_ino_t;
+
/* Are we big endian? */
/* XXX: Fatal assumption on Alpha platforms */
#define APR_IS_BIGENDIAN 0
Re: ABI change when _FILE_OFFSET_BITS=64 due to use of ino_t
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Lucian Adrian Grijincu wrote:
>
> But that's just the way I'd set things up, a vote should run to decide
> this things.
Consider that an @bug doxygen comment is a great way to mark something
as "must change" for apr 2.
Bill
Re: ABI change when _FILE_OFFSET_BITS=64 due to use of ino_t
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Lucian Adrian Grijincu wrote:
>
> But that's just the way I'd set things up, a vote should run to decide
> this things.
Consider that an @bug doxygen comment is a great way to mark something
as "must change" for apr 2.
Bill
Re: ABI change when _FILE_OFFSET_BITS=64 due to use of ino_t
Posted by Lucian Adrian Grijincu <lu...@gmail.com>.
On 10/12/07, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
> Lucian Adrian Grijincu wrote:
> >> I'm 99% certain that on win32 ino_t was always 32 bits, but I'm off
> >> to validate that assumption.
> >
> > quote from http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/include/apr_file_info.h?view=markup
> >
> > #if (defined WIN32) || (defined NETWARE)
> > /**
> > * Structure for determining the inode of the file.
> > */
> > typedef apr_uint64_t apr_ino_t;
> >
> > that looks like unsigned 64 bit wide interger to me.
>
> My bad, you are obviously correct! Shame on me for not looking at the
> code before opening my mouth :)
>
> >> Until 2.0 we obviously can't change the number of bits in apr_ino_t, period.
> >
> > Of course, no one was asking for such a change. We're preserving the
> > size that was present in APR until now, but we're making it not depend
> > on other flags.
>
> s/./ present at the client application's build time./
>
> yup!
yep, sorry for not making myself more explicit.
>
> > The width apr_ino_t had when libapr was compiled should be the same
> > width it has when it's being used in other projects. We're just trying
> > to enforce this by typedefing apr_ino_t to a type that is immutable
> > with respect to the value of _FILEOFFSET_BITS' value.
>
> Got it :)
>
> > And yeah, it seems reasonable that in 2.0 apr_ino_t==apr_uint64_t.
>
> You mean 1.2?
>
Nope. I think that in 2.0, to avoid any hassle apr_ino_t should have a
fixed size equal to the maximum used on all platforms. The two sizes
that are used (to my knowledge) are 4 and 8 bytes, thus
apr_ino_t==apr_uint64_t.
But that's just the way I'd set things up, a vote should run to decide
this things.
--
Lucian
Re: ABI change when _FILE_OFFSET_BITS=64 due to use of ino_t
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Lucian Adrian Grijincu wrote:
>> I'm 99% certain that on win32 ino_t was always 32 bits, but I'm off
>> to validate that assumption.
>
> quote from http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/include/apr_file_info.h?view=markup
>
> #if (defined WIN32) || (defined NETWARE)
> /**
> * Structure for determining the inode of the file.
> */
> typedef apr_uint64_t apr_ino_t;
>
> that looks like unsigned 64 bit wide interger to me.
My bad, you are obviously correct! Shame on me for not looking at the
code before opening my mouth :)
>> Until 2.0 we obviously can't change the number of bits in apr_ino_t, period.
>
> Of course, no one was asking for such a change. We're preserving the
> size that was present in APR until now, but we're making it not depend
> on other flags.
s/./ present at the client application's build time./
yup!
> The width apr_ino_t had when libapr was compiled should be the same
> width it has when it's being used in other projects. We're just trying
> to enforce this by typedefing apr_ino_t to a type that is immutable
> with respect to the value of _FILEOFFSET_BITS' value.
Got it :)
> And yeah, it seems reasonable that in 2.0 apr_ino_t==apr_uint64_t.
You mean 1.2?
Re: ABI change when _FILE_OFFSET_BITS=64 due to use of ino_t
Posted by Lucian Adrian Grijincu <lu...@gmail.com>.
On 10/12/07, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
> Joe Orton wrote:
> > Here's an updated patch:
> >
> > Index: configure.in
> > ===================================================================
> > --- configure.in (revision 583792)
> > +++ configure.in (working copy)
> > @@ -1456,6 +1456,21 @@
> > 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
> > +APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long, ino_t_value="unsigned long")
> > +AC_MSG_NOTICE([using $ino_t_value for ino_t])
>
> is it too early to actually compare ino_t to apr_uint64_t at this phase?
> In theory there will be embedded 32 bit int+long cases we should consider.
>
> > Index: include/apr.hw
> > ===================================================================
> > --- include/apr.hw (revision 583792)
> > +++ include/apr.hw (working copy)
> > @@ -346,6 +346,8 @@
> > #endif
> > typedef int apr_socklen_t;
> >
> > +typedef apr_uint64_t apr_ino_t;
> > +
>
> I'm 99% certain that on win32 ino_t was always 32 bits, but I'm off
> to validate that assumption.
>
quote from http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/include/apr_file_info.h?view=markup
#if (defined WIN32) || (defined NETWARE)
/**
* Structure for determining the inode of the file.
*/
typedef apr_uint64_t apr_ino_t;
/**
* Structure for determining the device the file is on.
*/
typedef apr_uint32_t apr_dev_t;
#else
/** The inode of the file. */
typedef ino_t apr_ino_t;
/**
* Structure for determining the device the file is on.
*/
typedef dev_t apr_dev_t;
#endif
that looks like unsigned 64 bit wide interger to me.
> Until 2.0 we obviously can't change the number of bits in apr_ino_t, period.
>
>
Of course, no one was asking for such a change. We're preserving the
size that was present in APR until now, but we're making it not depend
on other flags.
The width apr_ino_t had when libapr was compiled should be the same
width it has when it's being used in other projects. We're just trying
to enforce this by typedefing apr_ino_t to a type that is immutable
with respect to the value of _FILEOFFSET_BITS' value.
And yeah, it seems reasonable that in 2.0 apr_ino_t==apr_uint64_t.
--
Lucian
Re: ABI change when _FILE_OFFSET_BITS=64 due to use of ino_t
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> Here's an updated patch:
>
> Index: configure.in
> ===================================================================
> --- configure.in (revision 583792)
> +++ configure.in (working copy)
> @@ -1456,6 +1456,21 @@
> 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
> +APR_CHECK_TYPES_COMPATIBLE(ino_t, unsigned long, ino_t_value="unsigned long")
> +AC_MSG_NOTICE([using $ino_t_value for ino_t])
is it too early to actually compare ino_t to apr_uint64_t at this phase?
In theory there will be embedded 32 bit int+long cases we should consider.
> Index: include/apr.hw
> ===================================================================
> --- include/apr.hw (revision 583792)
> +++ include/apr.hw (working copy)
> @@ -346,6 +346,8 @@
> #endif
> typedef int apr_socklen_t;
>
> +typedef apr_uint64_t apr_ino_t;
> +
I'm 99% certain that on win32 ino_t was always 32 bits, but I'm off
to validate that assumption.
Until 2.0 we obviously can't change the number of bits in apr_ino_t, period.