diff mbox series

[1/2] fanotify: Move safe_fanotify_init() + safe macro into fanotify.h

Message ID 20200428113501.24711-1-pvorel@suse.cz
State Accepted
Headers show
Series [1/2] fanotify: Move safe_fanotify_init() + safe macro into fanotify.h | expand

Commit Message

Petr Vorel April 28, 2020, 11:35 a.m. UTC
Fanotify code is used only in testcases/kernel/syscalls/fanotify/, which
justify breaking rule of having safe function and macro outside of
library. This will be further cleaned in next commit.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi,

patchset is based on Amir's patchset [1], which I'd prefer to have
merged before.

Kind regards,
Petr

[1] https://patchwork.ozlabs.org/project/ltp/list/?series=171631&state=*

 include/tst_safe_macros.h                     |  5 ----
 lib/tst_safe_macros.c                         | 24 -----------------
 testcases/kernel/syscalls/fanotify/fanotify.h | 27 ++++++++++++++++++-
 3 files changed, 26 insertions(+), 30 deletions(-)

Comments

Cyril Hrubis Oct. 14, 2020, 1:46 p.m. UTC | #1
Hi!
> Fanotify code is used only in testcases/kernel/syscalls/fanotify/, which
> justify breaking rule of having safe function and macro outside of
> library. This will be further cleaned in next commit.

I guess that this is OK, I doubt that we will need a fanotify_init()
anywhere else but the fanotify testcases.
Petr Vorel Oct. 14, 2020, 6:02 p.m. UTC | #2
Hi,

> > Fanotify code is used only in testcases/kernel/syscalls/fanotify/, which
> > justify breaking rule of having safe function and macro outside of
> > library. This will be further cleaned in next commit.

> I guess that this is OK, I doubt that we will need a fanotify_init()
> anywhere else but the fanotify testcases.
Amir acked whole patch some time ago, thus I guess he's not against this first one.

The original motivation was to do further modifications for libc wrapper vs.
syscall() for fanotify_init() and fanotify_mark(). But I guess even without
second commit it makes sense to have all fanotify helpers together.
We can always revert it if really need them as safe macros.

Kind regards,
Petr
Li Wang Oct. 16, 2020, 3:20 a.m. UTC | #3
On Thu, Oct 15, 2020 at 2:02 AM Petr Vorel <pvorel@suse.cz> wrote:

> Hi,
>
> > > Fanotify code is used only in testcases/kernel/syscalls/fanotify/,
> which
> > > justify breaking rule of having safe function and macro outside of
> > > library. This will be further cleaned in next commit.
>
> > I guess that this is OK, I doubt that we will need a fanotify_init()
> > anywhere else but the fanotify testcases.
> Amir acked whole patch some time ago, thus I guess he's not against this
> first one.
>
> The original motivation was to do further modifications for libc wrapper
> vs.
> syscall() for fanotify_init() and fanotify_mark(). But I guess even without
> second commit it makes sense to have all fanotify helpers together.
> We can always revert it if really need them as safe macros.
>
+1

Reviewed-by: Li Wang <liwang@redhat.com>
Petr Vorel Oct. 16, 2020, 11:35 a.m. UTC | #4
Hi,

> > The original motivation was to do further modifications for libc wrapper
> > vs.
> > syscall() for fanotify_init() and fanotify_mark(). But I guess even without
> > second commit it makes sense to have all fanotify helpers together.
> > We can always revert it if really need them as safe macros.

> +1

> Reviewed-by: Li Wang <liwang@redhat.com>
Thanks Li! Merged.

I've also sent another fanotify cleanup patch:
https://patchwork.ozlabs.org/project/ltp/patch/20201016112441.4838-1-pvorel@suse.cz/

Kind regards,
Petr
diff mbox series

Patch

diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index c39d8768b..edaeef461 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -520,11 +520,6 @@  int safe_mincore(const char *file, const int lineno, void *start,
 #define SAFE_MINCORE(start, length, vec) \
 	safe_mincore(__FILE__, __LINE__, (start), (length), (vec))
 
-int safe_fanotify_init(const char *file, const int lineno,
-	unsigned int flags, unsigned int event_f_flags);
-#define SAFE_FANOTIFY_INIT(fan, mode)  \
-	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
-
 int safe_personality(const char *filename, unsigned int lineno,
 		    unsigned long persona);
 #define SAFE_PERSONALITY(persona) safe_personality(__FILE__, __LINE__, persona)
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index dbdfcc5be..9a9c73d06 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -45,29 +45,6 @@  pid_t safe_getpgid(const char *file, const int lineno, pid_t pid)
 	return pgid;
 }
 
-int safe_fanotify_init(const char *file, const int lineno,
-	unsigned int flags, unsigned int event_f_flags)
-{
-	int rval;
-
-#ifdef HAVE_SYS_FANOTIFY_H
-	rval = fanotify_init(flags, event_f_flags);
-
-	if (rval == -1) {
-		if (errno == ENOSYS) {
-			tst_brk(TCONF,
-				"fanotify is not configured in this kernel.");
-		}
-		tst_brk(TBROK | TERRNO,
-			"%s:%d: fanotify_init() failed", file, lineno);
-	}
-#else
-	tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
-#endif /* HAVE_SYS_FANOTIFY_H */
-
-	return rval;
-}
-
 int safe_personality(const char *filename, unsigned int lineno,
 		    unsigned long persona)
 {
@@ -96,7 +73,6 @@  int safe_setregid(const char *file, const int lineno,
 	return rval;
 }
 
-
 int safe_setreuid(const char *file, const int lineno,
 		  uid_t ruid, uid_t euid)
 {
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index a05f4a372..f74171c15 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -1,6 +1,6 @@ 
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
- * Copyright (c) 2012 Linux Test Project.  All Rights Reserved.
+ * Copyright (c) 2012-2020 Linux Test Project.  All Rights Reserved.
  * Author: Jan Kara, November 2013
  */
 
@@ -38,6 +38,31 @@  static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
 
 #endif /* HAVE_SYS_FANOTIFY_H */
 
+int safe_fanotify_init(const char *file, const int lineno,
+	unsigned int flags, unsigned int event_f_flags)
+{
+	int rval;
+
+#ifdef HAVE_SYS_FANOTIFY_H
+	rval = fanotify_init(flags, event_f_flags);
+
+	if (rval == -1) {
+		if (errno == ENOSYS) {
+			tst_brk(TCONF,
+				"fanotify is not configured in this kernel.");
+		}
+		tst_brk(TBROK | TERRNO,
+			"%s:%d: fanotify_init() failed", file, lineno);
+	}
+#else
+	tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
+#endif /* HAVE_SYS_FANOTIFY_H */
+
+	return rval;
+}
+#define SAFE_FANOTIFY_INIT(fan, mode)  \
+	safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
+
 #ifndef FAN_REPORT_TID
 #define FAN_REPORT_TID		0x00000100
 #endif