On 25 July 2012 23:10, Richard Henderson <r...@twiddle.net> wrote:
> For each target, only define the bits that appear in
> arch/target/include/asm/fcntl.h. Mirror the kernel's
> asm-generic layout by handling anything undefined afterward.
>
> Signed-off-by: Richard Henderson <r...@twiddle.net>
> ---
> linux-user/syscall_defs.h | 195 +++++++++++++++++++++-----------------------
> 1 files changed, 93 insertions(+), 102 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 02fe4f6..974d6f5 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2012,135 +2012,126 @@ struct target_statfs64 {
> #define TARGET_F_DUPFD_CLOEXEC (TARGET_F_LINUX_SPECIFIC_BASE + 6)
> #define TARGET_F_NOTIFY (TARGET_F_LINUX_SPECIFIC_BASE+2)
>
No entries for TARGET_M68K? I checked and I think that's the
only supported arch which isn't listed here and isn't only using
the asm-generic values. (It's the same 4 defines as TARGET_ARM.)
> -#if defined (TARGET_ARM)
> -#define TARGET_O_ACCMODE 0003
> -#define TARGET_O_RDONLY 00
> -#define TARGET_O_WRONLY 01
> -#define TARGET_O_RDWR 02
> -#define TARGET_O_CREAT 0100 /* not fcntl */
> -#define TARGET_O_EXCL 0200 /* not fcntl */
> -#define TARGET_O_NOCTTY 0400 /* not fcntl */
> -#define TARGET_O_TRUNC 01000 /* not fcntl */
> -#define TARGET_O_APPEND 02000
> -#define TARGET_O_NONBLOCK 04000
> -#define TARGET_O_NDELAY TARGET_O_NONBLOCK
> -#define TARGET_O_SYNC 010000
> -#define TARGET_FASYNC 020000 /* fcntl, for BSD compatibility */
> +#if defined(TARGET_ALPHA)
> +#define TARGET_O_NONBLOCK 04
> +#define TARGET_O_APPEND 010
> +#define TARGET_O_CREAT 01000 /* not fcntl */
> +#define TARGET_O_TRUNC 02000 /* not fcntl */
> +#define TARGET_O_EXCL 04000 /* not fcntl */
> +#define TARGET_O_NOCTTY 010000 /* not fcntl */
> +#define TARGET_FASYNC 020000 /* fcntl, for BSD compatibility */
> +#define TARGET_O_DSYNC 040000
> +#define TARGET_O_LARGEFILE 0 /* not necessary, always 64-bit */
> +#define TARGET_O_DIRECTORY 0100000 /* must be a directory */
> +#define TARGET_O_NOFOLLOW 0200000 /* don't follow links */
> +#define TARGET_O_DIRECT 02000000 /* direct disk access hint */
> +#define TARGET_O_NOATIME 04000000
> +#define TARGET_O_CLOEXEC 010000000
> +#define TARGET___O_SYNC 020000000
> +#define TARGET_O_PATH 040000000
The other targets were all straightforward to review but I had a
little difficulty with the Alpha bits here.
(1) O_LARGEFILE is 0 here but not in the kernel. Does the "not
necessary" comment here mean "not necessary for QEMU" ?
(ie is the difference because we are taking a different approach to
the kernel's "always set this bit on entry" one ?)
(2) where has the TARGET_FASYNC entry come from?
There are also a couple of hardcoded tabs lurking in some of
these lines which it would be nice to get rid of since we're
changing the code anyway. (checkpatch will tell you where they are.)
> +#elif defined (TARGET_ARM)
Not having the space between 'defined' and '(' would be consistent
with the TARGET_ALPHA one, be consistent with the lion's share of
defined() uses elswhere and also shut checkpatch up :-)
Otherwise good and I've checked all the numbers against the kernel
headers.
-- PMM
