diff mbox

libv4l: fixup lfs mismatch in preload libraries

Message ID 20180711132251.13172-1-ezequiel@collabora.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ezequiel Garcia July 11, 2018, 1:22 p.m. UTC 
From: Peter Korsgaard <jacmet@sunsite.dk>

Ensure that the lfs variants are not transparently used instead of the !lfs
ones so both can be wrapped, independently of any custom CFLAGS/CPPFLAGS.

Without this patch, the following assembler errors appear
during cross-compiling with Buildroot:

/tmp/ccc3gdJg.s: Assembler messages:
/tmp/ccc3gdJg.s:67: Error: symbol `open64' is already defined
/tmp/ccc3gdJg.s:130: Error: symbol `mmap64' is already defined

Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 lib/libv4l1/v4l1compat.c  | 3 +++
 lib/libv4l2/v4l2convert.c | 3 +++
 2 files changed, 6 insertions(+)

Comments

Mauro Carvalho Chehab July 11, 2018, 2:55 p.m. UTC | #1 
Em Wed, 11 Jul 2018 10:22:51 -0300
Ezequiel Garcia <ezequiel@collabora.com> escreveu:

> From: Peter Korsgaard <jacmet@sunsite.dk>
> 
> Ensure that the lfs variants are not transparently used instead of the !lfs
> ones so both can be wrapped, independently of any custom CFLAGS/CPPFLAGS.
> 
> Without this patch, the following assembler errors appear
> during cross-compiling with Buildroot:
> 
> /tmp/ccc3gdJg.s: Assembler messages:
> /tmp/ccc3gdJg.s:67: Error: symbol `open64' is already defined
> /tmp/ccc3gdJg.s:130: Error: symbol `mmap64' is already defined
> 
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  lib/libv4l1/v4l1compat.c  | 3 +++
>  lib/libv4l2/v4l2convert.c | 3 +++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
> index cb79629ff88f..e5c9e56261e2 100644
> --- a/lib/libv4l1/v4l1compat.c
> +++ b/lib/libv4l1/v4l1compat.c
> @@ -19,6 +19,9 @@
>  # Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA  02110-1335  USA
>   */
>  
> +/* ensure we see *64 variants and they aren't transparently used */
> +#undef _LARGEFILE_SOURCE
> +#undef _FILE_OFFSET_BITS

Hmm... shouldn't this be autodetected? I didn't check anything,
but I would be expecting that different distros (and BSD) may be
doing different things here, specially if they use different gcc
versions or even different libc implementations.


>  #define _LARGEFILE64_SOURCE 1
>  
>  #include <config.h>
> diff --git a/lib/libv4l2/v4l2convert.c b/lib/libv4l2/v4l2convert.c
> index 7c9a04c086ed..13ca4cfb1b08 100644
> --- a/lib/libv4l2/v4l2convert.c
> +++ b/lib/libv4l2/v4l2convert.c
> @@ -23,6 +23,9 @@
>  /* prevent GCC 4.7 inlining error */
>  #undef _FORTIFY_SOURCE
>  
> +/* ensure we see *64 variants and they aren't transparently used */
> +#undef _LARGEFILE_SOURCE
> +#undef _FILE_OFFSET_BITS
>  #define _LARGEFILE64_SOURCE 1
>  
>  #ifdef ANDROID



Thanks,
Mauro
Baruch Siach July 11, 2018, 7:38 p.m. UTC | #2 
Hi Mauro,

Added Peter's updated address to Cc.

Mauro Carvalho Chehab writes:
> Em Wed, 11 Jul 2018 10:22:51 -0300
> Ezequiel Garcia <ezequiel@collabora.com> escreveu:
>
>> From: Peter Korsgaard <jacmet@sunsite.dk>
>> 
>> Ensure that the lfs variants are not transparently used instead of the !lfs
>> ones so both can be wrapped, independently of any custom CFLAGS/CPPFLAGS.
>> 
>> Without this patch, the following assembler errors appear
>> during cross-compiling with Buildroot:
>> 
>> /tmp/ccc3gdJg.s: Assembler messages:
>> /tmp/ccc3gdJg.s:67: Error: symbol `open64' is already defined
>> /tmp/ccc3gdJg.s:130: Error: symbol `mmap64' is already defined
>> 
>> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> ---
>>  lib/libv4l1/v4l1compat.c  | 3 +++
>>  lib/libv4l2/v4l2convert.c | 3 +++
>>  2 files changed, 6 insertions(+)
>> 
>> diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
>> index cb79629ff88f..e5c9e56261e2 100644
>> --- a/lib/libv4l1/v4l1compat.c
>> +++ b/lib/libv4l1/v4l1compat.c
>> @@ -19,6 +19,9 @@
>>  # Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA  02110-1335  USA
>>   */
>>  
>> +/* ensure we see *64 variants and they aren't transparently used */
>> +#undef _LARGEFILE_SOURCE
>> +#undef _FILE_OFFSET_BITS
>
> Hmm... shouldn't this be autodetected? I didn't check anything,
> but I would be expecting that different distros (and BSD) may be
> doing different things here, specially if they use different gcc
> versions or even different libc implementations.

See Peter's explanation here:

  http://lists.busybox.net/pipermail/buildroot/2017-December/210067.html

baruch

>>  #define _LARGEFILE64_SOURCE 1
>>  
>>  #include <config.h>
>> diff --git a/lib/libv4l2/v4l2convert.c b/lib/libv4l2/v4l2convert.c
>> index 7c9a04c086ed..13ca4cfb1b08 100644
>> --- a/lib/libv4l2/v4l2convert.c
>> +++ b/lib/libv4l2/v4l2convert.c
>> @@ -23,6 +23,9 @@
>>  /* prevent GCC 4.7 inlining error */
>>  #undef _FORTIFY_SOURCE
>>  
>> +/* ensure we see *64 variants and they aren't transparently used */
>> +#undef _LARGEFILE_SOURCE
>> +#undef _FILE_OFFSET_BITS
>>  #define _LARGEFILE64_SOURCE 1
>>  
>>  #ifdef ANDROID
Ezequiel Garcia July 11, 2018, 10:57 p.m. UTC | #3 
On Wed, 2018-07-11 at 22:38 +0300, Baruch Siach wrote:
> Hi Mauro,
> 
> Added Peter's updated address to Cc.
> 
> Mauro Carvalho Chehab writes:
> > Em Wed, 11 Jul 2018 10:22:51 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> > 
> > > From: Peter Korsgaard <jacmet@sunsite.dk>
> > > 
> > > Ensure that the lfs variants are not transparently used instead
> > > of the !lfs
> > > ones so both can be wrapped, independently of any custom
> > > CFLAGS/CPPFLAGS.
> > > 
> > > Without this patch, the following assembler errors appear
> > > during cross-compiling with Buildroot:
> > > 
> > > /tmp/ccc3gdJg.s: Assembler messages:
> > > /tmp/ccc3gdJg.s:67: Error: symbol `open64' is already defined
> > > /tmp/ccc3gdJg.s:130: Error: symbol `mmap64' is already defined
> > > 
> > > Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > > ---
> > >  lib/libv4l1/v4l1compat.c  | 3 +++
> > >  lib/libv4l2/v4l2convert.c | 3 +++
> > >  2 files changed, 6 insertions(+)
> > > 
> > > diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
> > > index cb79629ff88f..e5c9e56261e2 100644
> > > --- a/lib/libv4l1/v4l1compat.c
> > > +++ b/lib/libv4l1/v4l1compat.c
> > > @@ -19,6 +19,9 @@
> > >  # Foundation, Inc., 51 Franklin Street, Suite 500, Boston,
> > > MA  02110-1335  USA
> > >   */
> > >  
> > > +/* ensure we see *64 variants and they aren't transparently used
> > > */
> > > +#undef _LARGEFILE_SOURCE
> > > +#undef _FILE_OFFSET_BITS
> > 
> > Hmm... shouldn't this be autodetected? I didn't check anything,
> > but I would be expecting that different distros (and BSD) may be
> > doing different things here, specially if they use different gcc
> > versions or even different libc implementations.
> 
> See Peter's explanation here:
> 
>   http://lists.busybox.net/pipermail/buildroot/2017-December/210067.h
> tml
> 
> 

Nice, thanks for Ccing the real Peter and for adding the link to the
discussion.

Regards,
Eze
Mauro Carvalho Chehab July 12, 2018, 8:54 a.m. UTC | #4 
Em Wed, 11 Jul 2018 22:38:56 +0300
Baruch Siach <baruch@tkos.co.il> escreveu:

> Hi Mauro,
> 
> Added Peter's updated address to Cc.
> 
> Mauro Carvalho Chehab writes:
> > Em Wed, 11 Jul 2018 10:22:51 -0300
> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
> >  
> >> From: Peter Korsgaard <jacmet@sunsite.dk>
> >> 
> >> Ensure that the lfs variants are not transparently used instead of the !lfs
> >> ones so both can be wrapped, independently of any custom CFLAGS/CPPFLAGS.
> >> 
> >> Without this patch, the following assembler errors appear
> >> during cross-compiling with Buildroot:
> >> 
> >> /tmp/ccc3gdJg.s: Assembler messages:
> >> /tmp/ccc3gdJg.s:67: Error: symbol `open64' is already defined
> >> /tmp/ccc3gdJg.s:130: Error: symbol `mmap64' is already defined
> >> 
> >> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> >> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> >> ---
> >>  lib/libv4l1/v4l1compat.c  | 3 +++
> >>  lib/libv4l2/v4l2convert.c | 3 +++
> >>  2 files changed, 6 insertions(+)
> >> 
> >> diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
> >> index cb79629ff88f..e5c9e56261e2 100644
> >> --- a/lib/libv4l1/v4l1compat.c
> >> +++ b/lib/libv4l1/v4l1compat.c
> >> @@ -19,6 +19,9 @@
> >>  # Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA  02110-1335  USA
> >>   */
> >>  
> >> +/* ensure we see *64 variants and they aren't transparently used */
> >> +#undef _LARGEFILE_SOURCE
> >> +#undef _FILE_OFFSET_BITS  
> >
> > Hmm... shouldn't this be autodetected? I didn't check anything,
> > but I would be expecting that different distros (and BSD) may be
> > doing different things here, specially if they use different gcc
> > versions or even different libc implementations.  
> 
> See Peter's explanation here:
> 
>   http://lists.busybox.net/pipermail/buildroot/2017-December/210067.html

The link Peter provided seems to be specific to glibc. The main
point I want to bring is: would this change affect users with
other setups? There are some users that compile it against FreeBSD
and Android. Some compile using dietlibc or uclibc. Also, people
build it against 32-bits and 64-bits on x86, arm and other archs.

So, the question is: are you sure that the above change is also valid for
*all* other environments? If not, I would be expecting it to be
attached to some automake test, to be sure that it will be applied
only to the affected setups.


> 
> baruch
> 
> >>  #define _LARGEFILE64_SOURCE 1
> >>  
> >>  #include <config.h>
> >> diff --git a/lib/libv4l2/v4l2convert.c b/lib/libv4l2/v4l2convert.c
> >> index 7c9a04c086ed..13ca4cfb1b08 100644
> >> --- a/lib/libv4l2/v4l2convert.c
> >> +++ b/lib/libv4l2/v4l2convert.c
> >> @@ -23,6 +23,9 @@
> >>  /* prevent GCC 4.7 inlining error */
> >>  #undef _FORTIFY_SOURCE
> >>  
> >> +/* ensure we see *64 variants and they aren't transparently used */
> >> +#undef _LARGEFILE_SOURCE
> >> +#undef _FILE_OFFSET_BITS
> >>  #define _LARGEFILE64_SOURCE 1
> >>  
> >>  #ifdef ANDROID  
> 
> 



Thanks,
Mauro
Baruch Siach July 12, 2018, 7:13 p.m. UTC | #5 
Mauro Carvalho Chehab writes:

> Em Wed, 11 Jul 2018 22:38:56 +0300
> Baruch Siach <baruch@tkos.co.il> escreveu:
>> Added Peter's updated address to Cc.
>>
>> Mauro Carvalho Chehab writes:
>> > Em Wed, 11 Jul 2018 10:22:51 -0300
>> > Ezequiel Garcia <ezequiel@collabora.com> escreveu:
>> >
>> >> From: Peter Korsgaard <jacmet@sunsite.dk>
>> >>
>> >> Ensure that the lfs variants are not transparently used instead of the !lfs
>> >> ones so both can be wrapped, independently of any custom CFLAGS/CPPFLAGS.
>> >>
>> >> Without this patch, the following assembler errors appear
>> >> during cross-compiling with Buildroot:
>> >>
>> >> /tmp/ccc3gdJg.s: Assembler messages:
>> >> /tmp/ccc3gdJg.s:67: Error: symbol `open64' is already defined
>> >> /tmp/ccc3gdJg.s:130: Error: symbol `mmap64' is already defined
>> >>
>> >> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>> >> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
>> >> ---
>> >>  lib/libv4l1/v4l1compat.c  | 3 +++
>> >>  lib/libv4l2/v4l2convert.c | 3 +++
>> >>  2 files changed, 6 insertions(+)
>> >>
>> >> diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
>> >> index cb79629ff88f..e5c9e56261e2 100644
>> >> --- a/lib/libv4l1/v4l1compat.c
>> >> +++ b/lib/libv4l1/v4l1compat.c
>> >> @@ -19,6 +19,9 @@
>> >>  # Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA  02110-1335  USA
>> >>   */
>> >>
>> >> +/* ensure we see *64 variants and they aren't transparently used */
>> >> +#undef _LARGEFILE_SOURCE
>> >> +#undef _FILE_OFFSET_BITS
>> >
>> > Hmm... shouldn't this be autodetected? I didn't check anything,
>> > but I would be expecting that different distros (and BSD) may be
>> > doing different things here, specially if they use different gcc
>> > versions or even different libc implementations.
>>
>> See Peter's explanation here:
>>
>>   http://lists.busybox.net/pipermail/buildroot/2017-December/210067.html
>
> The link Peter provided seems to be specific to glibc. The main
> point I want to bring is: would this change affect users with
> other setups? There are some users that compile it against FreeBSD
> and Android. Some compile using dietlibc or uclibc. Also, people
> build it against 32-bits and 64-bits on x86, arm and other archs.
>
> So, the question is: are you sure that the above change is also valid for
> *all* other environments? If not, I would be expecting it to be
> attached to some automake test, to be sure that it will be applied
> only to the affected setups.

Buildroot has been carrying this patch since 2012[1] with no one
complaining. Buildroot supports glibc, uClibc, and musl libc on a wide
range of architectures (x86, ARM, MIPS, PowerPC, Sparc, xtensa, arc, and
more) both 32-bit and 64-bit.

I don't know about FreeBSD.

baruch

[1] https://git.buildroot.org/buildroot/commit/?id=d059b1cc61dcafde47723c45b834409026a39dbe

>> >>  #define _LARGEFILE64_SOURCE 1
>> >>
>> >>  #include <config.h>
>> >> diff --git a/lib/libv4l2/v4l2convert.c b/lib/libv4l2/v4l2convert.c
>> >> index 7c9a04c086ed..13ca4cfb1b08 100644
>> >> --- a/lib/libv4l2/v4l2convert.c
>> >> +++ b/lib/libv4l2/v4l2convert.c
>> >> @@ -23,6 +23,9 @@
>> >>  /* prevent GCC 4.7 inlining error */
>> >>  #undef _FORTIFY_SOURCE
>> >>
>> >> +/* ensure we see *64 variants and they aren't transparently used */
>> >> +#undef _LARGEFILE_SOURCE
>> >> +#undef _FILE_OFFSET_BITS
>> >>  #define _LARGEFILE64_SOURCE 1
>> >>
>> >>  #ifdef ANDROID

--
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Peter Korsgaard July 12, 2018, 8:13 p.m. UTC | #6 
>>>>> "Baruch" == Baruch Siach <baruch@tkos.co.il> writes:

Hi,

 >> The link Peter provided seems to be specific to glibc. The main
 >> point I want to bring is: would this change affect users with
 >> other setups? There are some users that compile it against FreeBSD
 >> and Android. Some compile using dietlibc or uclibc. Also, people
 >> build it against 32-bits and 64-bits on x86, arm and other archs.
 >> 
 >> So, the question is: are you sure that the above change is also valid for
 >> *all* other environments? If not, I would be expecting it to be
 >> attached to some automake test, to be sure that it will be applied
 >> only to the affected setups.

 > Buildroot has been carrying this patch since 2012[1] with no one
 > complaining. Buildroot supports glibc, uClibc, and musl libc on a wide
 > range of architectures (x86, ARM, MIPS, PowerPC, Sparc, xtensa, arc, and
 > more) both 32-bit and 64-bit.

Indeed. I believe these comes from the single UNIX specification:

http://www.unix.org/version2/whatsnew/lfs20mar.html#3.3

And these defines are what the AC_SYS_LARGEFILE autoconf macro uses:

https://www.gnu.org/software/autoconf/manual/autoconf-2.65/html_node/System-Services.html
diff mbox

Patch

diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
index cb79629ff88f..e5c9e56261e2 100644
--- a/lib/libv4l1/v4l1compat.c
+++ b/lib/libv4l1/v4l1compat.c
@@ -19,6 +19,9 @@ 
 # Foundation, Inc., 51 Franklin Street, Suite 500, Boston, MA  02110-1335  USA
  */
 
+/* ensure we see *64 variants and they aren't transparently used */
+#undef _LARGEFILE_SOURCE
+#undef _FILE_OFFSET_BITS
 #define _LARGEFILE64_SOURCE 1
 
 #include <config.h>
diff --git a/lib/libv4l2/v4l2convert.c b/lib/libv4l2/v4l2convert.c
index 7c9a04c086ed..13ca4cfb1b08 100644
--- a/lib/libv4l2/v4l2convert.c
+++ b/lib/libv4l2/v4l2convert.c
@@ -23,6 +23,9 @@ 
 /* prevent GCC 4.7 inlining error */
 #undef _FORTIFY_SOURCE
 
+/* ensure we see *64 variants and they aren't transparently used */
+#undef _LARGEFILE_SOURCE
+#undef _FILE_OFFSET_BITS
 #define _LARGEFILE64_SOURCE 1
 
 #ifdef ANDROID