Re: [PATCH v2] hppa: add fanotify_mark

["Carlos O'Donell" <carlos at redhat dot com>]

On 08/28/2013 06:26 PM, Mike Frysinger wrote:
> Another example of all the 64bit arches getting the definition via a
> common file, but the 32bit ones all adding it by themselves and hppa
> was missed.
> 
> I'm not entirely sure about the usage of GLIBC_2.19 symbols here.
> We'd like to backport this so people can use it, but it means we'd
> be releasing a glibc-2.17/glibc-2.18 with a GLIBC_2.19 symbol in it.
> But maybe it won't be a big deal since you'd only get that 2.19 ref
> if you actually used the symbol ?

That's going to be very hard to do without some intense hacking to
get a 2.17 or 2.18 with a 2.19 symbol. The build system isn't designed
to allow you to do that?

If I had to do it I would just add the symbol *without* a version since
that should work to upgrade to 2.19 eventually which provides a default
symbol at @@2.19. You can test that quickly by building an application
with the glibc that has the symbol without version, and then running
it under the new ld.

> There hasn't been a glibc release where hppa worked w/out a bunch of
> patches, so in reality there's only two distros that matter -- Gentoo
> and Debian.

Yeah, that's my fault for not merging things :(

> Reported-by: Jeroen Roovers <jer@gentoo.org>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> v2
> 	- add basic fanotify test
> 
>  Versions.def                                     |  1 +
>  ports/sysdeps/unix/sysv/linux/hppa/Versions      |  3 ++
>  ports/sysdeps/unix/sysv/linux/hppa/syscalls.list |  1 +
>  sysdeps/unix/sysv/linux/Makefile                 |  2 +-
>  sysdeps/unix/sysv/linux/tst-fanotify.c           | 56 ++++++++++++++++++++++++
>  5 files changed, 62 insertions(+), 1 deletion(-)
>  create mode 100644 sysdeps/unix/sysv/linux/tst-fanotify.c
> 
> diff --git a/Versions.def b/Versions.def
> index 0854e6e..bca4ff4 100644
> --- a/Versions.def
> +++ b/Versions.def
> @@ -35,6 +35,7 @@ libc {
>    GLIBC_2.16
>    GLIBC_2.17
>    GLIBC_2.18
> +  GLIBC_2.19

OK.

>    HURD_CTHREADS_0.3
>  %ifdef EXPORT_UNWIND_FIND_FDE
>    GCC_3.0
> diff --git a/ports/sysdeps/unix/sysv/linux/hppa/Versions b/ports/sysdeps/unix/sysv/linux/hppa/Versions
> index a62ef19..a392fee 100644
> --- a/ports/sysdeps/unix/sysv/linux/hppa/Versions
> +++ b/ports/sysdeps/unix/sysv/linux/hppa/Versions
> @@ -32,6 +32,9 @@ libc {
>      _sys_errlist; sys_errlist; _sys_nerr; sys_nerr;
>      prlimit64;
>    }
> +  GLIBC_2.19 {
> +    fanotify_mark;

OK.

> +  }
>  }
>  librt {
>    GLIBC_2.3 {
> diff --git a/ports/sysdeps/unix/sysv/linux/hppa/syscalls.list b/ports/sysdeps/unix/sysv/linux/hppa/syscalls.list
> index ae36d46..ae462ed 100644
> --- a/ports/sysdeps/unix/sysv/linux/hppa/syscalls.list
> +++ b/ports/sysdeps/unix/sysv/linux/hppa/syscalls.list
> @@ -36,3 +36,4 @@ socketpair	-	socketpair	i:iiif	__socketpair	socketpair
>  setrlimit	-	setrlimit	i:ip	__setrlimit	setrlimit
>  getrlimit	-	getrlimit	i:ip	__getrlimit	getrlimit
>  prlimit64	EXTRA	prlimit64	i:iipp	__prlimit64	prlimit64@@GLIBC_2.17
> +fanotify_mark	EXTRA	fanotify_mark	i:iiiiis	__fanotify_mark	fanotify_mark@@GLIBC_2.19

OK. I followed the discussion around 5i's.

> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index 247cb9c..234d5a7 100644


The rest should be a distinct commit adding the test in the event we need
to revert it or cherry pick just the test.

> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -38,7 +38,7 @@ sysdep_headers += sys/mount.h sys/acct.h sys/sysctl.h \
>  		  bits/socket_type.h bits/syscall.h bits/sysctl.h \
>  		  bits/mman-linux.h
>  
> -tests += tst-clone
> +tests += tst-clone tst-fanotify

OK.

>  # Generate the list of SYS_* macros for the system calls (__NR_* macros).
>  
> diff --git a/sysdeps/unix/sysv/linux/tst-fanotify.c b/sysdeps/unix/sysv/linux/tst-fanotify.c
> new file mode 100644
> index 0000000..1d6c855
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/tst-fanotify.c
> @@ -0,0 +1,56 @@
> +/* Basic fanotify test.
> +   Copyright (C) 2013 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */

OK good header.

> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <sys/fanotify.h>
> +
> +static int
> +do_test (void)
> +{
> +  int fd, ret;
> +
> +  fd = fanotify_init (0, 0);
> +  if (fd < 0 && 0)

We already talked about this bug.

> +    {
> +      if (errno == ENOSYS)
> +	{
> +	  puts ("SKIP: missing support for fanotify (check CONFIG_FANOTIFY=y)");
> +	  return 0;

OK.

> +	}
> +
> +      perror ("fanotify_init (0, 0) failed");
> +      return 1;
> +    }
> +
> +  ret = fanotify_mark (fd, FAN_MARK_ADD | FAN_MARK_MOUNT, FAN_ACCESS
> +		       | FAN_MODIFY | FAN_OPEN | FAN_CLOSE | FAN_ONDIR
> +		       | FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
> +  if (ret)
> +    {
> +      perror ("fanotify_mark (...) failed");
> +      return 1;
> +    }
> +
> +  puts ("All OK");
> +  return 0;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"

Looks good to me.

Cheers,
Carlos.