[Xorg-driver-geode] [PATCH video-geode] Replace LFS transitional extension API lseek64 and off64_t
Gaetan Nadon
memsize at videotron.ca
Tue Nov 15 08:08:50 PST 2011
On Tue, 2011-11-15 at 10:27 +0200, Martin-Éric Racine wrote:
> Gaetan,
>
> Thanks for this patch.
>
> I'm really wondering whether _LARGEFILE64_SOURCE was intentionally
> defined or not i.e. do the Durango and MSR codes require explicit
> 64-bit support or not. IIRC there were similar defines in the Cimarron
> code as well. Let's see what the AMD guys have to say.
It'd be nice to know if the geode really needs large file descriptor
indeed.
If not, we simply remove AC_SYS_LARGEFILE from the path and we are done.
I scanned 50 xorg video drivers and found only one (ati) with
AC_SYS_LARGEFILE
but I don't think it uses any transitional extension xxx64 functions.
Only a hand full of xorg modules out of 240 uses large file support.
I doubt the code can handle large file support as it seems to me that it
is "hard coded" for 32 bit.
GeodeReadMSR(unsigned long addr, unsigned long *lo, unsigned long *hi)
{
unsigned int data[2];
int fd = _msr_open();
int ret;
if (fd == -1)
return -1;
ret = lseek(fd, (off_t) addr, SEEK_SET);
[...]
It is casting an unsigned long type (32 bit) to an off_t type (which
could be 64 bit). It also does not check for an overflow condition which
it should do when supporting 64 bit seek operations on a 32 bit system.
I am no expert in this area.
>
> Basically, if this clears ok with them, the only thing we'd need in
> addition to this to support arbitrary ports would be conditionaly
> enabling compilation of the z4l.c driver upon detection of the
> header's presence.
>
> Martin-Éric
>
> 2011/11/14 Gaetan Nadon <memsize at videotron.ca>:
> > The "transitional extension" API is deprecated and is not available on FreeBSD.
> > It gets replaced with defining _FILE_OFFSET_BITS.
> > http://www.unix.org/version2/whatsnew/lfs20mar.html#3.0
> >
> > _LARGEFILE64_SOURCE
> > Expose definitions for the alternative API specified by the LFS (Large
> > File Summit) as a "transitional extension" to the Single UNIX
> > Specification. (See http://opengroup.org/platform/lfs.html.) The
> > alternative API consists of a set of new objects (i.e., functions and
> > types) whose names are suffixed with "64" (e.g., off64_t versus off_t,
> > lseek64() versus lseek(), etc.). New programs should not employ this
> > interface; instead _FILE_OFFSET_BITS=64 should be employed.
> >
> > _FILE_OFFSET_BITS
> > Defining this macro with the value 64 automatically converts references
> > to 32-bit functions and data types related to file I/O and file system
> > operations into references to their 64-bit counterparts. This is
> > useful for performing I/O on large files (> 2 Gigabytes) on 32-bit
> > systems. (Defining this macro permits correctly written programs to
> > use large files with only a recompilation being required.) 64-bit
> > systems naturally permit file sizes greater than 2 Gigabytes, and on
> > those systems this macro has no effect.
> >
> > An example using open():
> > -----------------------------------------------------------------------
> > int
> > main (void)
> > {
> > return open (".", O_RDONLY) < 0;
> > }
> >
> > it should compile and run just fine, with either Sun C or GCC. In the
> > latter case, the return statement expands via the preprocessor to:
> >
> > return open64 (".", 0) < 0;
> >
> > so you don't need to have any instances of open64 in your code.
> > -------------------------------------------------------------------------
> >
> > The file _FILE_OFFSET_BITS macro will be defined by AC_SYS_LARGEFILE
> > which gets included by the generated config.h.
> >
> > This is consistent with all X.Org modules using large file support,
> > including the server. Recently large file support was added to xorg
> > app/sessreg which uses lseek.
> > Only AC_SYS_LARGEFILE was added in sessreg configure.ac.
> >
> > It is still unsure if geode driver need support for large file descriptor >2G
> >
> > Signed-off-by: Gaetan Nadon <memsize at videotron.ca>
> > ---
> > configure.ac | 1 +
> > src/durango.c | 2 --
> > src/geode_msr.c | 9 ++++++---
> > 3 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index 483caff..31e5fe0 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -30,6 +30,7 @@ AC_INIT([xf86-video-geode],
> > AC_CONFIG_SRCDIR([Makefile.am])
> > AM_CONFIG_HEADER([config.h])
> > AC_CONFIG_AUX_DIR(.)
> > +AC_SYS_LARGEFILE
> >
> > # Require xorg-macros: XORG_DEFAULT_OPTIONS
> > m4_ifndef([XORG_MACROS_VERSION],
> > diff --git a/src/durango.c b/src/durango.c
> > index 9d6970b..8795d41 100644
> > --- a/src/durango.c
> > +++ b/src/durango.c
> > @@ -32,8 +32,6 @@
> > #include "config.h"
> > #endif
> >
> > -#define _LARGEFILE64_SOURCE
> > -
> > #include <unistd.h>
> > #include <errno.h>
> > #include <compiler.h>
> > diff --git a/src/geode_msr.c b/src/geode_msr.c
> > index 6de693f..26fd78f 100644
> > --- a/src/geode_msr.c
> > +++ b/src/geode_msr.c
> > @@ -1,4 +1,7 @@
> > -#define _LARGEFILE64_SOURCE
> > +#ifdef HAVE_CONFIG_H
> > +#include "config.h"
> > +#endif
> > +
> > #include <stdio.h>
> > #include <unistd.h>
> > #include <fcntl.h>
> > @@ -31,7 +34,7 @@ GeodeReadMSR(unsigned long addr, unsigned long *lo, unsigned long *hi)
> > if (fd == -1)
> > return -1;
> >
> > - ret = lseek64(fd, (off64_t) addr, SEEK_SET);
> > + ret = lseek(fd, (off_t) addr, SEEK_SET);
> >
> > if (ret == -1)
> > return -1;
> > @@ -56,7 +59,7 @@ GeodeWriteMSR(unsigned long addr, unsigned long lo, unsigned long hi)
> > if (fd == -1)
> > return -1;
> >
> > - if (lseek64(fd, (off64_t) addr, SEEK_SET) == -1)
> > + if (lseek(fd, (off_t) addr, SEEK_SET) == -1)
> > return -1;
> >
> > data[0] = lo;
> > --
> > 1.7.4.1
> >
> > _______________________________________________
> > Xorg-driver-geode mailing list
> > Xorg-driver-geode at lists.x.org
> > http://lists.x.org/mailman/listinfo/xorg-driver-geode
> >
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-driver-geode/attachments/20111115/0991ff82/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-driver-geode/attachments/20111115/0991ff82/attachment-0001.pgp>
More information about the Xorg-driver-geode
mailing list