[PATCH v1 0/6] Small fio cleanups and fixes

[PATCH v1 0/6] Small fio cleanups and fixes

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

This series contains small cleanups and fixes:

- Patch 1 is just a trivial indentation fix.
- Patch 2, 3 are to add `ENOMEM` case error handling.
- Patch 4 is to replace `malloc()`+`memset()` with `calloc()`.
- Patch 5 is just a small optimization for json.
- Patch 6 is to fix clang-15 warning when compiling autogenerated file, lex.yy.c.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Ammar Faizi (6):
  backend: Fix indentation
  get_cgroup_root: Handle `ENOMEM` case on `malloc()` call
  stat: Handle `ENOMEM` case on `malloc()` call
  engines/net: Replace `malloc()`+`memset()` with `calloc()`
  json: Change `if (!strlen(str))` to `if (!str[0])`
  Makefile: Suppress `-Wimplicit-fallthrough` when compiling `lex.yy`

 Makefile      |  6 +++++-
 backend.c     |  2 +-
 cgroup.c      |  4 ++++
 engines/net.c | 10 ++++++----
 json.c        |  2 +-
 stat.c        | 13 +++++++++++++
 6 files changed, 30 insertions(+), 7 deletions(-)


base-commit: 5f2d43188c2d65674aaba6280e2a87107e5d7099
-- 
Ammar Faizi

[PATCH v1 1/6] backend: Fix indentation

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/backend.c b/backend.c
index 317e4f6c..071fd1d1 100644
--- a/backend.c
+++ b/backend.c
@@ -2021,7 +2021,7 @@ static void reap_threads(unsigned int *nr_running, uint64_t *t_rate,
 	for_each_td(td, i) {
 		int flags = 0;
 
-		 if (!strcmp(td->o.ioengine, "cpuio"))
+		if (!strcmp(td->o.ioengine, "cpuio"))
 			cputhreads++;
 		else
 			realthreads++;
-- 
Ammar Faizi

[PATCH v1 2/6] get_cgroup_root: Handle `ENOMEM` case on `malloc()` call

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Add error handling on malloc() call to prevent a NULL pointer
dereference.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 cgroup.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cgroup.c b/cgroup.c
index 77e31a4d..b559b70f 100644
--- a/cgroup.c
+++ b/cgroup.c
@@ -114,6 +114,8 @@ void cgroup_kill(struct flist_head *clist)
 static char *get_cgroup_root(struct thread_data *td, struct cgroup_mnt *mnt)
 {
 	char *str = malloc(64);
+	if (!str)
+		return NULL;
 
 	if (td->o.cgroup)
 		sprintf(str, "%s/%s", mnt->path, td->o.cgroup);
@@ -178,6 +180,8 @@ int cgroup_setup(struct thread_data *td, struct flist_head *clist, struct cgroup
 	 * Create container, if it doesn't exist
 	 */
 	root = get_cgroup_root(td, *mnt);
+	if (!root)
+		return 1;
 	if (mkdir(root, 0755) < 0) {
 		int __e = errno;
 
-- 
Ammar Faizi

[PATCH v1 3/6] stat: Handle `ENOMEM` case on `malloc()` call

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Add error handling on malloc() call to prevent a NULL pointer
dereference.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 stat.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/stat.c b/stat.c
index 949af5ed..2d74c3b0 100644
--- a/stat.c
+++ b/stat.c
@@ -2430,6 +2430,10 @@ void __show_run_stats(void)
 	struct flist_head **opt_lists;
 
 	runstats = malloc(sizeof(struct group_run_stats) * (groupid + 1));
+	if (!runstats) {
+		log_err("fio: failed to allocate runstats\n");
+		return;
+	}
 
 	for (i = 0; i < groupid + 1; i++)
 		init_group_run_stat(&runstats[i]);
@@ -2455,7 +2459,16 @@ void __show_run_stats(void)
 	}
 
 	threadstats = malloc(nr_ts * sizeof(struct thread_stat));
+	if (!threadstats) {
+		log_err("fio: failed to allocate threadstats\n");
+		return;
+	}
+
 	opt_lists = malloc(nr_ts * sizeof(struct flist_head *));
+	if (!opt_lists) {
+		log_err("fio: failed to allocate opt_lists\n");
+		return;
+	}
 
 	for (i = 0; i < nr_ts; i++) {
 		init_thread_stat(&threadstats[i]);
-- 
Ammar Faizi

[PATCH v1 4/6] engines/net: Replace `malloc()`+`memset()` with `calloc()`

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Replace `malloc()` + `memset()` with `calloc()`. While in there, handle
the ENOMEM case of it.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 engines/net.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/engines/net.c b/engines/net.c
index c6cec584..8a898748 100644
--- a/engines/net.c
+++ b/engines/net.c
@@ -1363,6 +1363,10 @@ static int fio_netio_setup(struct thread_data *td)
 {
 	struct netio_data *nd;
 
+	nd = calloc(1, sizeof(*nd));
+	if (!nd)
+		return 1;
+
 	if (!td->files_index) {
 		add_file(td, td->o.filename ?: "net", 0, 0);
 		td->o.nr_files = td->o.nr_files ?: 1;
@@ -1370,9 +1374,6 @@ static int fio_netio_setup(struct thread_data *td)
 	}
 
 	if (!td->io_ops_data) {
-		nd = malloc(sizeof(*nd));
-
-		memset(nd, 0, sizeof(*nd));
 		nd->listenfd = -1;
 		nd->pipes[0] = nd->pipes[1] = -1;
 		td->io_ops_data = nd;
@@ -1391,7 +1392,8 @@ static int fio_netio_setup_splice(struct thread_data *td)
 {
 	struct netio_data *nd;
 
-	fio_netio_setup(td);
+	if (fio_netio_setup(td))
+		return 1;
 
 	nd = td->io_ops_data;
 	if (nd) {
-- 
Ammar Faizi

[PATCH v1 5/6] json: Change `if (!strlen(str))` to `if (!str[0])`

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

No need to traverse the whole string. Using `!strlen(str)` as a
conditional expression is effectively the same with `!str[0]`.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/json.c b/json.c
index cd3d5d74..8b650721 100644
--- a/json.c
+++ b/json.c
@@ -56,7 +56,7 @@ static char *strdup_escape(const char *str)
 	char *p, *ret;
 	int escapes;
 
-	if (!strlen(str))
+	if (!str[0])
 		return NULL;
 
 	escapes = 0;
-- 
Ammar Faizi

[PATCH v1 6/6] Makefile: Suppress `-Wimplicit-fallthrough` when compiling `lex.yy`

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

lex.yy.c is an auto generated C file. When compiling with clang-15, we
got the following warning:

```
      CC lex.yy.o
  lex.yy.c:1444:5: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
                                  case EOB_ACT_END_OF_FILE:
                                  ^
  lex.yy.c:1444:5: note: insert '__attribute__((fallthrough));' to silence this warning
                                  case EOB_ACT_END_OF_FILE:
                                  ^
                                  __attribute__((fallthrough));
  lex.yy.c:1444:5: note: insert 'break;' to avoid fall-through
                                  case EOB_ACT_END_OF_FILE:
                                  ^
                                  break;
  1 warning generated.
```

There is nothing we can do to fix lex.yy.c since it's an auto generated
file. Append `-Wno-implicit-fallthrough` when compiling this file if we
have -Wimplicit-fallthrough enabled.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 Makefile | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e670c1f2..1e15a69e 100644
--- a/Makefile
+++ b/Makefile
@@ -530,8 +530,12 @@ else
 	$(QUIET_LEX)$(LEX) $<
 endif
 
+ifneq (,$(findstring -Wimplicit-fallthrough,$(CFLAGS)))
+LEX_YY_CFLAGS := -Wno-implicit-fallthrough
+endif
+
 lex.yy.o: lex.yy.c y.tab.h
-	$(QUIET_CC)$(CC) -o $@ $(CFLAGS) $(CPPFLAGS) -c $<
+	$(QUIET_CC)$(CC) -o $@ $(CFLAGS) $(CPPFLAGS) $(LEX_YY_CFLAGS) -c $<
 
 y.tab.o: y.tab.c y.tab.h
 	$(QUIET_CC)$(CC) -o $@ $(CFLAGS) $(CPPFLAGS) -c $<
-- 
Ammar Faizi

Re: [PATCH v1 3/6] stat: Handle `ENOMEM` case on `malloc()` call

[Ammar Faizi]

On 4/27/22 4:20 AM, Ammar Faizi wrote:
>   	threadstats = malloc(nr_ts * sizeof(struct thread_stat));
> +	if (!threadstats) {
> +		log_err("fio: failed to allocate threadstats\n");
> +		return;
> +	}
> +
>   	opt_lists = malloc(nr_ts * sizeof(struct flist_head *));
> +	if (!opt_lists) {
> +		log_err("fio: failed to allocate opt_lists\n");

I should free(threadstats) here, will send v2 soon.

> +		return;
> +	}
>   
>   	for (i = 0; i < nr_ts; i++) {
>   		init_thread_stat(&threadstats[i]);

-- 
Ammar Faizi

Re: [PATCH v1 4/6] engines/net: Replace `malloc()`+`memset()` with `calloc()`

[Ammar Faizi]

On 4/27/22 4:20 AM, Ammar Faizi wrote:
> diff --git a/engines/net.c b/engines/net.c
> index c6cec584..8a898748 100644
> --- a/engines/net.c
> +++ b/engines/net.c
> @@ -1363,6 +1363,10 @@ static int fio_netio_setup(struct thread_data *td)
>   {
>   	struct netio_data *nd;
>   
> +	nd = calloc(1, sizeof(*nd));
> +	if (!nd)
> +		return 1;

This is also a wrong calloc() placement, will fix that in v2. I will
make sure to self-review carefully before send this time.

>   	if (!td->files_index) {
>   		add_file(td, td->o.filename ?: "net", 0, 0);
>   		td->o.nr_files = td->o.nr_files ?: 1;
> @@ -1370,9 +1374,6 @@ static int fio_netio_setup(struct thread_data *td)
>   	}
>   
>   	if (!td->io_ops_data) {
> -		nd = malloc(sizeof(*nd));
> -
> -		memset(nd, 0, sizeof(*nd));
>   		nd->listenfd = -1;
>   		nd->pipes[0] = nd->pipes[1] = -1;
>   		td->io_ops_data = nd;
> @@ -1391,7 +1392,8 @@ static int fio_netio_setup_splice(struct thread_data *td)
>   {
>   	struct netio_data *nd;
>   
> -	fio_netio_setup(td);
> +	if (fio_netio_setup(td))
> +		return 1;
>   
>   	nd = td->io_ops_data;
>   	if (nd) {


-- 
Ammar Faizi