[PATCH] posix_favise{64} error handling fixes [was Re: fadvise gclibc vs uclibc]

Carmelo AMOROSO carmelo.amoroso at st.com
Thu Sep 18 13:29:09 UTC 2008


Bernhard Reutner-Fischer wrote:
> On Mon, Sep 15, 2008 at 01:55:56PM +0200, Carmelo AMOROSO wrote:
>> Corinna Schultz wrote:
>>> Quoting Carmelo AMOROSO <carmelo.amoroso at st.com>:
>>>> a colleague of mine is right now working to produce a patch for
>>>> posix_fadvise to fix all LTP tests using posix_fadvise[64].
>>>>
>>>> Indeed LTP tests expect that, when posix_fadvise[64] fails,
>>>> it should return as return value an error code (-errno) instead
>>>> of simply setting properly errno and returning -1.
>>> Did you see my earlier message, detailing the errors I'm seeing? I have 
>>> very little experience with this low-level programming, and don't 
>>> really know how to begin fixing it, so if you have people already 
>>> working on it, I'll happily wait for your patch. :) Do you have an 
>>> estimate of when your patch will be available?
>>>
>>> -Corinna
>>>
>>>
>>>
>> Hi Corinna, may you try the attached patch.
>> It worked fine for NPTL branch solving all LTP posix_fadvise tests.
>> Let me know, so we can enqueue for commit.
>>
>> Thanks to Filippo for having fixed this.
>>
>> Cheers,
>> Carmelo
> 
>> This patch fixes posix_fadvise[64] function to return the 
>> error number in case of failure instead of -1 and setting errno,
>> according to SuSv3 (IEEE Std 1003.1 2004 edition) specification.
>> Add INTERNAL_SYSCALL macros to libc/sysdeps/linux/sh/bits/syscalls.h
>> to align sh to other archs.
>>
>> Signed-off-by: Filippo Arcidiacono <filippo.arcidiacono at st.com>
>> Reviewed-by:   Carmelo Amoroso <carmelo.amoroso at st.com>
>>
>> Index: libc/sysdeps/linux/common/posix_fadvise64.c
>> ===================================================================
>> --- libc/sysdeps/linux/common/posix_fadvise64.c	(revision 23401)
>> +++ libc/sysdeps/linux/common/posix_fadvise64.c	(working copy)
>> @@ -40,14 +40,35 @@
>>   return INTERNAL_SYSCALL_ERRNO (ret, err);
>> }
>> #else
>> -_syscall4(int, posix_fadvise64, int, fd, __off64_t, offset,
>> +static int syscall_posix_fadvise(int fd, off_t offset1, off_t offset2, off_t len, int advice);
>> +#define __NR_syscall_posix_fadvise64 __NR_posix_fadvise64
>> +_syscall4(int, syscall_posix_fadvise64, int, fd, __off64_t, offset,
> 
> I would have said static inline, like most of the rest does. Doesn't
> matter though (but we should sync them up to one scheme, i guess).
> 

Absolutely agreed. IIRC I should now use __inline__ keyword, right?

>>           __off64_t, len, int, advice);
>> +int posix_fadvise64(int fd, __off64_t offset, __off64_t len, int advice)
>> +{
>> +	int ret = syscall_posix_fadvise64(fd, offset, len, advice);
>> +	if (ret == -1)
>> +		return errno;
>> +	return ret;
>> +}
>> #endif
>>
>> /* 32 bit implementation is kind of a pita */
>> #elif __WORDSIZE == 32
>>
>> -#ifdef _syscall6 /* workaround until everyone has _syscall6() */
>> +#ifndef INTERNAL_SYSCALL
>> +int posix_fadvise64(int fd, __off64_t offset, __off64_t len, int advice)
>> +{
>> +	INTERNAL_SYSCALL_DECL (err);
>> +	int ret = INTERNAL_SYSCALL (fadvise64_64, err, 6, fd,
> 
> Not sure.
> #ifndef INTERNAL_SYSCALL
> [clip]
> ret INTERNAL_SYSCALL(...)
> 
> I suppose the 'n' was not intended.
> 
You're right. It was a temporary change for testing the other case too.
Sorry for that.

>> --- libc/sysdeps/linux/common/posix_fadvise.c	(revision 23401)
>> +++ libc/sysdeps/linux/common/posix_fadvise.c	(working copy)
>> @@ -33,8 +33,18 @@
>>     return 0;
>> }
>> #else
>> -_syscall4(int, posix_fadvise, int, fd, off_t, offset,
>> -          off_t, len, int, advice);
>> +static int syscall_posix_fadvise(int fd, off_t offset1, off_t offset2, off_t len, int advice);
>> +#define __NR_syscall_posix_fadvise __NR_fadvise64
>> +_syscall5(int, syscall_posix_fadvise, int, fd, off_t, offset1,
>> +          off_t, offset2, off_t, len, int, advice);
>> +		  
> 
> whitespace damage above will be cought by the pre-ci hook, so i don't
> need to mention it now. hm.. ;)
> 
sorry. fixed
> Please fix the CPP logic and check it in.
> TIA,
> Bernhard
> 
Carmelo




More information about the uClibc mailing list