On Wed, May 18, 2016 at 12:21:46PM +0100, Catalin Marinas wrote:

On Tue, May 17, 2016 at 10:05:26PM +0300, Yury Norov wrote:

quoted

On Mon, May 16, 2016 at 06:06:05PM +0100, Catalin Marinas wrote:

quoted

On Sat, May 14, 2016 at 06:03:52PM +0300, Yury Norov wrote:

quoted

+SYSCALL_DEFINE6(mmap2, unsigned long, addr, unsigned long, len,
+       unsigned long, prot, unsigned long, flags, unsigned long, fd,
+       unsigned long, pgoff)

To avoid the types confusion we could add __SC_WRAP to mmap2 in unistd.h
and use COMPAT_SYSCALL_DEFINE6 here (together with compat_ptr_t etc.).

quoted

+{
+       if (pgoff & (~PAGE_MASK >> 12))
+               return -EINVAL;
+
+       return sys_mmap_pgoff((compat_uptr_t) addr, (compat_size_t) len,
+		       (int) prot, (int) flags, (int) fd,
+		       pgoff >> (PAGE_SHIFT-12));

Then we wouldn't need the explicit casting here.

See below

quoted

quoted

+}
+
+COMPAT_SYSCALL_DEFINE4(pread64, unsigned int, fd, compat_uptr_t __user *, ubuf,
+		       compat_size_t, count, off_t, offset)
+{
+	return sys_pread64(fd, (char *) ubuf, count, offset);
+}
+
+COMPAT_SYSCALL_DEFINE4(pwrite64, unsigned int, fd, compat_uptr_t __user *, ubuf,
+		       compat_size_t, count, off_t, offset)
+{
+	return sys_pwrite64(fd, (char *) ubuf, count, offset);

Nitpick: no space between cast type and variable name: (char *)ubuf, ...

I think it's really a matter of taste. I prefer to have a space, and
there's no solid rule in coding style.

And there are 13032 insertions of my version vs 35030 of yours:
~/work/linux$ git grep ' \*)[a-zA-Z]'|wc -l
35030
~/work/linux$ git grep ' \*) [a-zA-Z]'|wc -l
13032

Of course, I will change it if you insist.

Not really, I thought it's covered by CodingStyle but it doesn't seem
to.

quoted

quoted

We can also make these functions static as they are not used outside
this file.

If it's OK, we can use just compat_sys_xxx() instead of
COMPAT_SYSCALL_DEFINEx(xxx),

I got lost in macros, what difference would COMPAT_SYSCALL_DEFINE vs
compat_sys_*() make? Is this the delouse stuff?

Hmm... I looked again. It seems that COMPAT delouses all arguments,
including off_t, and that's what we try to avoid. So we *should* use
naked functions. 

include/linux/compat.h:
53 #define COMPAT_SYSCALL_DEFINEx(x, name, ...) \
54         asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))\
55              __attribute__((alias(__stringify(compat_SyS##name))));  \
56         static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
57         asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__));\
58         asmlinkage long compat_SyS##name(__MAP(x,__SC_LONG,__VA_ARGS__))\
59         { \
60                 return C_SYSC##name(__MAP(x,__SC_DELOUSE,__VA_ARGS__)); \
61         } \
62         static inline long C_SYSC##name(__MAP(x,__SC_DECL,__VA_ARGS__))

quoted

and for mmap2 we'll not need to change _SYSCALL to _SC_WRAP in
unistd.h that way.

Looking at the generic unistd.h, adding _SC_WRAP for sys_mmap2 is
indeed not easy:

#define __NR3264_mmap 222
__SC_3264(__NR3264_mmap, sys_mmap2, sys_mmap)

So defining a compat_sys_mmap2() would work but I think you'd also need:

#define sys_mmap2	compat_sys_mmap2()

OK.

-- 
Catalin