diff mbox series

[02/13] lib: Replace path_exist() with tst_path_exists()

Message ID 20241218184518.16190-3-chrubis@suse.cz
State Changes Requested
Headers show
Series Get rid of testcases/kernel/mem/lib library | expand

Commit Message

Cyril Hrubis Dec. 18, 2024, 6:45 p.m. UTC
Move and rename the path_exist() function from
testcases/kernel/mem/lib/ to the to level library.

This removes mem.h dependency from mem/cpuset/ test.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_fs.h                       | 10 ++++++++++
 lib/tst_path_exists.c                  | 23 +++++++++++++++++++++++
 testcases/kernel/mem/cpuset/Makefile   |  2 +-
 testcases/kernel/mem/cpuset/cpuset01.c |  4 ++--
 testcases/kernel/mem/include/mem.h     |  1 -
 testcases/kernel/mem/lib/mem.c         | 18 +++---------------
 6 files changed, 39 insertions(+), 19 deletions(-)
 create mode 100644 lib/tst_path_exists.c

Comments

Petr Vorel Dec. 19, 2024, 2:31 p.m. UTC | #1
Hi Cyril,

> Move and rename the path_exist() function from
> testcases/kernel/mem/lib/ to the to level library.
+1

> This removes mem.h dependency from mem/cpuset/ test.
+1

> diff --git a/include/tst_fs.h b/include/tst_fs.h
> index 835f3511c..f6ac6a40d 100644
> --- a/include/tst_fs.h
> +++ b/include/tst_fs.h
> @@ -145,6 +145,16 @@ int tst_dir_is_empty_(void (*cleanup)(void), const char *name, int verbose);
>   */
>  int tst_get_path(const char *prog_name, char *buf, size_t buf_len);

> +/**
> + * tst_path_exists()
nit: any short desc?

> + *
> + * @param fmt A printf-like format used to construct the path.
> + * @param ... A printf-like parameter list.
> + * @return Non-zero if path exists, zero otherwise.
> + */

Thanks for taking care of the docs. I guess this is doxygen syntax right?
Could you please before merge fix doc syntax - use kernel doc formatting? (no
param, return: and add short desc for the function).

/**
 * tst_path_exists() - check path exists
 *
 * @fmt: A printf-like format used to construct the path.
 * @... A printf-like parameter list.
 * return: Non-zero if path exists, zero otherwise.
 */

This will apply at least to the first commit as well.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr
Cyril Hrubis Dec. 19, 2024, 3:04 p.m. UTC | #2
Hi!
> > diff --git a/include/tst_fs.h b/include/tst_fs.h
> > index 835f3511c..f6ac6a40d 100644
> > --- a/include/tst_fs.h
> > +++ b/include/tst_fs.h
> > @@ -145,6 +145,16 @@ int tst_dir_is_empty_(void (*cleanup)(void), const char *name, int verbose);
> >   */
> >  int tst_get_path(const char *prog_name, char *buf, size_t buf_len);
> 
> > +/**
> > + * tst_path_exists()
> nit: any short desc?

I kind of do not like repeating the same sentence we have in the return description here.

> > + *
> > + * @param fmt A printf-like format used to construct the path.
> > + * @param ... A printf-like parameter list.
> > + * @return Non-zero if path exists, zero otherwise.
> > + */
> 
> Thanks for taking care of the docs. I guess this is doxygen syntax right?
> Could you please before merge fix doc syntax - use kernel doc formatting? (no
> param, return: and add short desc for the function).

Sigh, yes. Will fix.
Petr Vorel Dec. 20, 2024, 8:02 a.m. UTC | #3
> Hi!
> > > diff --git a/include/tst_fs.h b/include/tst_fs.h
> > > index 835f3511c..f6ac6a40d 100644
> > > --- a/include/tst_fs.h
> > > +++ b/include/tst_fs.h
> > > @@ -145,6 +145,16 @@ int tst_dir_is_empty_(void (*cleanup)(void), const char *name, int verbose);
> > >   */
> > >  int tst_get_path(const char *prog_name, char *buf, size_t buf_len);

> > > +/**
> > > + * tst_path_exists()
> > nit: any short desc?

> I kind of do not like repeating the same sentence we have in the return description here.

Sure. It triggers warning:

[kernel-doc WARN] : missing initial short description of 'tst_path_exists'

I'll just ignore them (some warning mean wrong docs, this one is innocent,
also we have 145x warnings about duplicate label, thus it's not easy to see if
newly added source has proper syntax or not).

> > > + *
> > > + * @param fmt A printf-like format used to construct the path.
> > > + * @param ... A printf-like parameter list.
> > > + * @return Non-zero if path exists, zero otherwise.
> > > + */

> > Thanks for taking care of the docs. I guess this is doxygen syntax right?
> > Could you please before merge fix doc syntax - use kernel doc formatting? (no
> > param, return: and add short desc for the function).

> Sigh, yes. Will fix.

Thanks (this mean missing docs).

Kind regards,
Petr
diff mbox series

Patch

diff --git a/include/tst_fs.h b/include/tst_fs.h
index 835f3511c..f6ac6a40d 100644
--- a/include/tst_fs.h
+++ b/include/tst_fs.h
@@ -145,6 +145,16 @@  int tst_dir_is_empty_(void (*cleanup)(void), const char *name, int verbose);
  */
 int tst_get_path(const char *prog_name, char *buf, size_t buf_len);
 
+/**
+ * tst_path_exists()
+ *
+ * @param fmt A printf-like format used to construct the path.
+ * @param ... A printf-like parameter list.
+ * @return Non-zero if path exists, zero otherwise.
+ */
+int tst_path_exists(const char *fmt, ...)
+    __attribute__ ((format (printf, 1, 2)));
+
 /*
  * Fill a file with specified pattern
  * @fd: file descriptor
diff --git a/lib/tst_path_exists.c b/lib/tst_path_exists.c
new file mode 100644
index 000000000..333c4b0e5
--- /dev/null
+++ b/lib/tst_path_exists.c
@@ -0,0 +1,23 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) Linux Test Project, 2011-2021
+ * Copyright (c) Cyril Hrubis <chrubis@suse.cz> 2024
+ */
+
+#include <stdio.h>
+#include <stdarg.h>
+#include <unistd.h>
+#include <limits.h>
+#include "tst_fs.h"
+
+int tst_path_exists(const char *fmt, ...)
+{
+        va_list ap;
+        char pathbuf[PATH_MAX];
+
+        va_start(ap, fmt);
+        vsnprintf(pathbuf, sizeof(pathbuf), fmt, ap);
+        va_end(ap);
+
+        return access(pathbuf, F_OK) == 0;
+}
diff --git a/testcases/kernel/mem/cpuset/Makefile b/testcases/kernel/mem/cpuset/Makefile
index 8e41c0223..bac13e02b 100644
--- a/testcases/kernel/mem/cpuset/Makefile
+++ b/testcases/kernel/mem/cpuset/Makefile
@@ -20,5 +20,5 @@ 
 top_srcdir		?= ../../../..
 
 include $(top_srcdir)/include/mk/testcases.mk
-include $(top_srcdir)/testcases/kernel/mem/include/libmem.mk
+include $(top_srcdir)/testcases/kernel/include/lib.mk
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/mem/cpuset/cpuset01.c b/testcases/kernel/mem/cpuset/cpuset01.c
index 956ac30c8..4f4415a18 100644
--- a/testcases/kernel/mem/cpuset/cpuset01.c
+++ b/testcases/kernel/mem/cpuset/cpuset01.c
@@ -30,7 +30,7 @@ 
 #include <numaif.h>
 #endif
 
-#include "mem.h"
+#include "tst_test.h"
 #include "numa_helper.h"
 
 #ifdef HAVE_NUMA_V2
@@ -164,7 +164,7 @@  static long count_cpu(void)
 {
 	int ncpus = 0;
 
-	while (path_exist(PATH_SYS_SYSTEM "/cpu/cpu%d", ncpus))
+	while (tst_path_exists(PATH_SYS_SYSTEM "/cpu/cpu%d", ncpus))
 		ncpus++;
 
 	return ncpus;
diff --git a/testcases/kernel/mem/include/mem.h b/testcases/kernel/mem/include/mem.h
index 865d2c7e8..87528fdd0 100644
--- a/testcases/kernel/mem/include/mem.h
+++ b/testcases/kernel/mem/include/mem.h
@@ -70,7 +70,6 @@  void write_cpusets(const struct tst_cg_group *cg, long nd);
 
 /* shared */
 unsigned int get_a_numa_node(void);
-int  path_exist(const char *path, ...);
 
 void update_shm_size(size_t *shm_size);
 
diff --git a/testcases/kernel/mem/lib/mem.c b/testcases/kernel/mem/lib/mem.c
index 3e0f5d1bb..f293e766e 100644
--- a/testcases/kernel/mem/lib/mem.c
+++ b/testcases/kernel/mem/lib/mem.c
@@ -545,19 +545,19 @@  static void gather_node_cpus(char *cpus, long nd)
 	char buf[BUFSIZ];
 	char path[BUFSIZ], path1[BUFSIZ];
 
-	while (path_exist(PATH_SYS_SYSTEM "/cpu/cpu%d", ncpus))
+	while (tst_path_exists(PATH_SYS_SYSTEM "/cpu/cpu%d", ncpus))
 		ncpus++;
 
 	for (i = 0; i < ncpus; i++) {
 		snprintf(path, BUFSIZ,
 			 PATH_SYS_SYSTEM "/node/node%ld/cpu%d", nd, i);
-		if (path_exist(path)) {
+		if (tst_path_exists("%s", path)) {
 			snprintf(path1, BUFSIZ, "%s/online", path);
 			/*
 			 * if there is no online knob, then the cpu cannot
 			 * be taken offline
 			 */
-			if (path_exist(path1)) {
+			if (tst_path_exists("%s", path1)) {
 				SAFE_FILE_SCANF(path1, "%ld", &online);
 				if (online == 0)
 					continue;
@@ -626,18 +626,6 @@  unsigned int get_a_numa_node(void)
 	abort();
 }
 
-int path_exist(const char *path, ...)
-{
-	va_list ap;
-	char pathbuf[PATH_MAX];
-
-	va_start(ap, path);
-	vsnprintf(pathbuf, sizeof(pathbuf), path, ap);
-	va_end(ap);
-
-	return access(pathbuf, F_OK) == 0;
-}
-
 void update_shm_size(size_t * shm_size)
 {
 	size_t shmmax;