Patchwork libc: posix_fadvise: Fix build breakage for !LFS

login
register
mail settings
Submitter Vineet Gupta
Date Jan. 9, 2014, 9:35 a.m.
Message ID <1389260145-8810-1-git-send-email-vgupta@synopsys.com>
Download mbox | patch
Permalink /patch/308533/
State New
Headers show

Comments

Vineet Gupta - Jan. 9, 2014, 9:35 a.m.
commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa"
enabled posix_fadvise() for all arches (it was just not generated
before).

However this also unearthed an issue introduced by ee84b8b400
"linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to
referencing LFS'ish code (off64_t) w/o proper checks which causes build
to break for !LFS.

Fix this by calling posix_fadvise64() only for LFS case and open-code
it's equivalent for !LFS.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: Baruch Siach <baruch@tkos.co.il>
Cc: Khem Raj <raj.khem@gmail.com>
Cc: markos Chandras <markos.chandras@gmail.com>
Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
---
 libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)
Vineet Gupta - Feb. 4, 2014, 4:32 a.m.
Ping !

On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote:
> commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa"
> enabled posix_fadvise() for all arches (it was just not generated
> before).
> 
> However this also unearthed an issue introduced by ee84b8b400
> "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to
> referencing LFS'ish code (off64_t) w/o proper checks which causes build
> to break for !LFS.
> 
> Fix this by calling posix_fadvise64() only for LFS case and open-code
> it's equivalent for !LFS.
> 
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Cc: Baruch Siach <baruch@tkos.co.il>
> Cc: Khem Raj <raj.khem@gmail.com>
> Cc: markos Chandras <markos.chandras@gmail.com>
> Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
> ---
>  libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
> index 25c294178e5e..14bbeeea13bc 100644
> --- a/libc/sysdeps/linux/common/posix_fadvise.c
> +++ b/libc/sysdeps/linux/common/posix_fadvise.c
> @@ -22,17 +22,34 @@
>  # include <endian.h>
>  # include <bits/wordsize.h>
>  
> -# ifdef __NR_fadvise64_64
> -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
> -# endif
> +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__)
> +#include <_lfs_64.h>
>  
> +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
>  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>  {
> -# ifdef __NR_fadvise64_64
>  	return posix_fadvise64(fd, offset, len, advice);
> -# else
> +}
> +#else
> +
> +int posix_fadvise(int fd, off_t offset, off_t len, int advice)
> +{
>  	int ret;
>  	INTERNAL_SYSCALL_DECL(err);
> +
> +# ifdef __NR_fadvise64_64
> +#  if __WORDSIZE == 64
> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice);
> +#  else
> +#   if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__)
> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice,
> +			OFF_HI_LO (offset), OFF_HI_LO (len));
> +#   else
> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd,
> +			OFF_HI_LO (offset), OFF_HI_LO (len), advice);
> +#   endif
> +#  endif
> +# else  /* __NR_fadvise64 */
>  #  if __WORDSIZE == 64
>  	ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice);
>  #  else
> @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>  #   endif
>  			OFF_HI_LO (offset), len, advice);
>  #  endif
> +#  endif
>  	if (INTERNAL_SYSCALL_ERROR_P (ret, err))
>  		return INTERNAL_SYSCALL_ERRNO (ret, err);
>  	return 0;
> -#  endif
>  }
>  # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
>  strong_alias(posix_fadvise,posix_fadvise64)
>  # endif
>  #endif
> +#endif
>
Vineet Gupta - April 1, 2014, 5:23 a.m.
Ping ^ 2

On Tuesday 04 February 2014 10:02 AM, Vineet Gupta wrote:
> Ping !
> 
> On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote:
>> commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa"
>> enabled posix_fadvise() for all arches (it was just not generated
>> before).
>>
>> However this also unearthed an issue introduced by ee84b8b400
>> "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to
>> referencing LFS'ish code (off64_t) w/o proper checks which causes build
>> to break for !LFS.
>>
>> Fix this by calling posix_fadvise64() only for LFS case and open-code
>> it's equivalent for !LFS.
>>
>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>> Cc: Mike Frysinger <vapier@gentoo.org>
>> Cc: Baruch Siach <baruch@tkos.co.il>
>> Cc: Khem Raj <raj.khem@gmail.com>
>> Cc: markos Chandras <markos.chandras@gmail.com>
>> Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
>> ---
>>  libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------
>>  1 file changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
>> index 25c294178e5e..14bbeeea13bc 100644
>> --- a/libc/sysdeps/linux/common/posix_fadvise.c
>> +++ b/libc/sysdeps/linux/common/posix_fadvise.c
>> @@ -22,17 +22,34 @@
>>  # include <endian.h>
>>  # include <bits/wordsize.h>
>>  
>> -# ifdef __NR_fadvise64_64
>> -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
>> -# endif
>> +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__)
>> +#include <_lfs_64.h>
>>  
>> +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
>>  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>>  {
>> -# ifdef __NR_fadvise64_64
>>  	return posix_fadvise64(fd, offset, len, advice);
>> -# else
>> +}
>> +#else
>> +
>> +int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>> +{
>>  	int ret;
>>  	INTERNAL_SYSCALL_DECL(err);
>> +
>> +# ifdef __NR_fadvise64_64
>> +#  if __WORDSIZE == 64
>> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice);
>> +#  else
>> +#   if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__)
>> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice,
>> +			OFF_HI_LO (offset), OFF_HI_LO (len));
>> +#   else
>> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd,
>> +			OFF_HI_LO (offset), OFF_HI_LO (len), advice);
>> +#   endif
>> +#  endif
>> +# else  /* __NR_fadvise64 */
>>  #  if __WORDSIZE == 64
>>  	ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice);
>>  #  else
>> @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>>  #   endif
>>  			OFF_HI_LO (offset), len, advice);
>>  #  endif
>> +#  endif
>>  	if (INTERNAL_SYSCALL_ERROR_P (ret, err))
>>  		return INTERNAL_SYSCALL_ERRNO (ret, err);
>>  	return 0;
>> -#  endif
>>  }
>>  # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
>>  strong_alias(posix_fadvise,posix_fadvise64)
>>  # endif
>>  #endif
>> +#endif
>>
aldot - June 12, 2014, 4:10 p.m.
On Tue, Apr 01, 2014 at 10:53:29AM +0530, Vineet Gupta wrote:
> Ping ^ 2
> 
> On Tuesday 04 February 2014 10:02 AM, Vineet Gupta wrote:
> > Ping !
> > 
> > On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote:
> >> commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa"
> >> enabled posix_fadvise() for all arches (it was just not generated
> >> before).
> >>
> >> However this also unearthed an issue introduced by ee84b8b400
> >> "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to
> >> referencing LFS'ish code (off64_t) w/o proper checks which causes build
> >> to break for !LFS.
> >>
> >> Fix this by calling posix_fadvise64() only for LFS case and open-code
> >> it's equivalent for !LFS.

I do not like the open-coding, can we avoid it?
I.e. let the __NR_fadvise64_64 based impl live in posix_fadvise64.c,
__WORDSIZE == 64 ?

thanks,
> >>
> >> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> >> Cc: Mike Frysinger <vapier@gentoo.org>
> >> Cc: Baruch Siach <baruch@tkos.co.il>
> >> Cc: Khem Raj <raj.khem@gmail.com>
> >> Cc: markos Chandras <markos.chandras@gmail.com>
> >> Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
> >> ---
> >>  libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------
> >>  1 file changed, 24 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
> >> index 25c294178e5e..14bbeeea13bc 100644
> >> --- a/libc/sysdeps/linux/common/posix_fadvise.c
> >> +++ b/libc/sysdeps/linux/common/posix_fadvise.c
> >> @@ -22,17 +22,34 @@
> >>  # include <endian.h>
> >>  # include <bits/wordsize.h>
> >>  
> >> -# ifdef __NR_fadvise64_64
> >> -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
> >> -# endif
> >> +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__)
> >> +#include <_lfs_64.h>
> >>  
> >> +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
> >>  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
> >>  {
> >> -# ifdef __NR_fadvise64_64
> >>  	return posix_fadvise64(fd, offset, len, advice);
> >> -# else
> >> +}
> >> +#else
> >> +
> >> +int posix_fadvise(int fd, off_t offset, off_t len, int advice)
> >> +{
> >>  	int ret;
> >>  	INTERNAL_SYSCALL_DECL(err);
> >> +
> >> +# ifdef __NR_fadvise64_64
> >> +#  if __WORDSIZE == 64
> >> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice);
> >> +#  else
> >> +#   if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__)
> >> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice,
> >> +			OFF_HI_LO (offset), OFF_HI_LO (len));
> >> +#   else
> >> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd,
> >> +			OFF_HI_LO (offset), OFF_HI_LO (len), advice);
> >> +#   endif
> >> +#  endif
> >> +# else  /* __NR_fadvise64 */
> >>  #  if __WORDSIZE == 64
> >>  	ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice);
> >>  #  else
> >> @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice)
> >>  #   endif
> >>  			OFF_HI_LO (offset), len, advice);
> >>  #  endif
> >> +#  endif
> >>  	if (INTERNAL_SYSCALL_ERROR_P (ret, err))
> >>  		return INTERNAL_SYSCALL_ERRNO (ret, err);
> >>  	return 0;
> >> -#  endif
> >>  }
> >>  # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
> >>  strong_alias(posix_fadvise,posix_fadvise64)
> >>  # endif
> >>  #endif
> >> +#endif
> >>
> 
> _______________________________________________
> uClibc mailing list
> uClibc@uclibc.org
> http://lists.busybox.net/mailman/listinfo/uclibc
Vineet Gupta - July 22, 2014, 2:33 p.m.
On Thursday 12 June 2014 09:40 PM, Bernhard Reutner-Fischer wrote:
> On Tue, Apr 01, 2014 at 10:53:29AM +0530, Vineet Gupta wrote:
>> Ping ^ 2
>>
>> On Tuesday 04 February 2014 10:02 AM, Vineet Gupta wrote:
>>> Ping !
>>>
>>> On Thursday 09 January 2014 03:05 PM, Vineet Gupta wrote:
>>>> commit 00571b43df2e "libc: posix_fadvise: restore implementation for xtensa"
>>>> enabled posix_fadvise() for all arches (it was just not generated
>>>> before).
>>>>
>>>> However this also unearthed an issue introduced by ee84b8b400
>>>> "linux: posix_fadvise: use new SYSCALL_ALIGN_64BIT" which is to
>>>> referencing LFS'ish code (off64_t) w/o proper checks which causes build
>>>> to break for !LFS.
>>>>
>>>> Fix this by calling posix_fadvise64() only for LFS case and open-code
>>>> it's equivalent for !LFS.
> 
> I do not like the open-coding, can we avoid it?

I don't like it too however I don't see how it will work. W/o LFS we can't call
any *64 stuff.

> I.e. let the __NR_fadvise64_64 based impl live in posix_fadvise64.c,
> __WORDSIZE == 64 ?

I don't think I understand.

1. My patch doesn't touch wordsize == 64 case, it handles the size 32 case.
2. I agree that current code is open coding posix_fadvise64 inside posix_fadvise,
but I don't know a better way of doing it - ideas / suggestions welcome !

Thx,
-Vineet

> 
> thanks,
>>>>
>>>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>>>> Cc: Mike Frysinger <vapier@gentoo.org>
>>>> Cc: Baruch Siach <baruch@tkos.co.il>
>>>> Cc: Khem Raj <raj.khem@gmail.com>
>>>> Cc: markos Chandras <markos.chandras@gmail.com>
>>>> Cc: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
>>>> ---
>>>>  libc/sysdeps/linux/common/posix_fadvise.c | 30 ++++++++++++++++++++++++------
>>>>  1 file changed, 24 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
>>>> index 25c294178e5e..14bbeeea13bc 100644
>>>> --- a/libc/sysdeps/linux/common/posix_fadvise.c
>>>> +++ b/libc/sysdeps/linux/common/posix_fadvise.c
>>>> @@ -22,17 +22,34 @@
>>>>  # include <endian.h>
>>>>  # include <bits/wordsize.h>
>>>>  
>>>> -# ifdef __NR_fadvise64_64
>>>> -int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
>>>> -# endif
>>>> +# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__)
>>>> +#include <_lfs_64.h>
>>>>  
>>>> +int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
>>>>  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>>>>  {
>>>> -# ifdef __NR_fadvise64_64
>>>>  	return posix_fadvise64(fd, offset, len, advice);
>>>> -# else
>>>> +}
>>>> +#else
>>>> +
>>>> +int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>>>> +{
>>>>  	int ret;
>>>>  	INTERNAL_SYSCALL_DECL(err);
>>>> +
>>>> +# ifdef __NR_fadvise64_64
>>>> +#  if __WORDSIZE == 64
>>>> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice);
>>>> +#  else
>>>> +#   if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__)
>>>> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice,
>>>> +			OFF_HI_LO (offset), OFF_HI_LO (len));
>>>> +#   else
>>>> +	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd,
>>>> +			OFF_HI_LO (offset), OFF_HI_LO (len), advice);
>>>> +#   endif
>>>> +#  endif
>>>> +# else  /* __NR_fadvise64 */
>>>>  #  if __WORDSIZE == 64
>>>>  	ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice);
>>>>  #  else
>>>> @@ -43,12 +60,13 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice)
>>>>  #   endif
>>>>  			OFF_HI_LO (offset), len, advice);
>>>>  #  endif
>>>> +#  endif
>>>>  	if (INTERNAL_SYSCALL_ERROR_P (ret, err))
>>>>  		return INTERNAL_SYSCALL_ERRNO (ret, err);
>>>>  	return 0;
>>>> -#  endif
>>>>  }
>>>>  # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
>>>>  strong_alias(posix_fadvise,posix_fadvise64)
>>>>  # endif
>>>>  #endif
>>>> +#endif
>>>>
>>
>> _______________________________________________
>> uClibc mailing list
>> uClibc@uclibc.org
>> http://lists.busybox.net/mailman/listinfo/uclibc

Patch

diff --git a/libc/sysdeps/linux/common/posix_fadvise.c b/libc/sysdeps/linux/common/posix_fadvise.c
index 25c294178e5e..14bbeeea13bc 100644
--- a/libc/sysdeps/linux/common/posix_fadvise.c
+++ b/libc/sysdeps/linux/common/posix_fadvise.c
@@ -22,17 +22,34 @@ 
 # include <endian.h>
 # include <bits/wordsize.h>
 
-# ifdef __NR_fadvise64_64
-int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
-# endif
+# if defined(__NR_fadvise64_64) && defined(__UCLIBC_HAS_LFS__)
+#include <_lfs_64.h>
 
+int posix_fadvise64(int fd, off64_t offset, off64_t len, int advice);
 int posix_fadvise(int fd, off_t offset, off_t len, int advice)
 {
-# ifdef __NR_fadvise64_64
 	return posix_fadvise64(fd, offset, len, advice);
-# else
+}
+#else
+
+int posix_fadvise(int fd, off_t offset, off_t len, int advice)
+{
 	int ret;
 	INTERNAL_SYSCALL_DECL(err);
+
+# ifdef __NR_fadvise64_64
+#  if __WORDSIZE == 64
+	ret = INTERNAL_SYSCALL(fadvise64_64, err, 4, fd, offset, len, advice);
+#  else
+#   if defined(__UCLIBC_SYSCALL_ALIGN_64BIT__) || defined(__arm__)
+	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd, advice,
+			OFF_HI_LO (offset), OFF_HI_LO (len));
+#   else
+	ret = INTERNAL_SYSCALL(fadvise64_64, err, 6, fd,
+			OFF_HI_LO (offset), OFF_HI_LO (len), advice);
+#   endif
+#  endif
+# else  /* __NR_fadvise64 */
 #  if __WORDSIZE == 64
 	ret = INTERNAL_SYSCALL(fadvise64, err, 4, fd, offset, len, advice);
 #  else
@@ -43,12 +60,13 @@  int posix_fadvise(int fd, off_t offset, off_t len, int advice)
 #   endif
 			OFF_HI_LO (offset), len, advice);
 #  endif
+#  endif
 	if (INTERNAL_SYSCALL_ERROR_P (ret, err))
 		return INTERNAL_SYSCALL_ERRNO (ret, err);
 	return 0;
-#  endif
 }
 # if defined __UCLIBC_HAS_LFS__ && (!defined __NR_fadvise64_64 || __WORDSIZE == 64)
 strong_alias(posix_fadvise,posix_fadvise64)
 # endif
 #endif
+#endif