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

Bernhard Reutner-Fischer rep.dot.nop at gmail.com
Thu Sep 18 13:10:41 UTC 2008


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).

>           __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.

>--- 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.. ;)

Please fix the CPP logic and check it in.
TIA,
Bernhard



More information about the uClibc mailing list