[LTP] [PATCH] cfs-scheduler: Fixed "make check" errors and warnings.
Petr Vorel
pvorel@suse.cz
Tue Apr 7 11:54:18 CEST 2026
Hi Samir,
> Fixed all make check errors and warnings in cfs_bandwidth01.c and
> hackbench.c to comply with LTP coding style.
I guess you get that info with make check (which is using checkpatch.pl and
other code).
> cfs_bandwidth01.c:
> - Remove initialization of static variable to 0
> hackbench.c:
FYI hackbench.c would deserve to rewrite into new C API. With that many of your
fixes would be not needed because the old code would be deleted.
> - Add SPDX-License-Identifier header
> - Remove FSF mailing address paragraph
> - Remove filename from file header
> - Remove initialization of static variables to 0
> - Convert zero-length array to C99 flexible array
> - Modify barf() function to accept variadic arguments
> - Use __func__ instead of hardcoded function names
> - Separate assignments from if conditions
> - Fix pointer declaration spacing
> - Add blank line after declarations
> - Fix spacing in macro and struct initialization
> - Remove unnecessary braces for single statement
> Both files now pass make check validation with 0 errors and 0 warnings.
> Signed-off-by: Samir <samir@linux.ibm.com>
> ---
> .../sched/cfs-scheduler/cfs_bandwidth01.c | 2 +-
> .../kernel/sched/cfs-scheduler/hackbench.c | 121 +++++++++---------
> 2 files changed, 58 insertions(+), 65 deletions(-)
> diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> index e52858f8e..8c511f060 100644
> --- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> +++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
> @@ -35,7 +35,7 @@
> static struct tst_cg_group *cg_level2, *cg_level3a, *cg_level3b;
> static struct tst_cg_group *cg_workers[3];
> -static int may_have_waiters = 0;
> +static int may_have_waiters;
+1
> static void set_cpu_quota(const struct tst_cg_group *const cg,
> const float quota_percent)
> diff --git a/testcases/kernel/sched/cfs-scheduler/hackbench.c b/testcases/kernel/sched/cfs-scheduler/hackbench.c
> index 6f37060aa..6a30e1cc8 100644
> --- a/testcases/kernel/sched/cfs-scheduler/hackbench.c
> +++ b/testcases/kernel/sched/cfs-scheduler/hackbench.c
> @@ -1,51 +1,33 @@
> -/******************************************************************************/
> -/* Copyright Rusty Russell, */
> -/* Copyright Pierre Peiffer */
> -/* Copyright Zhang, Yanmin, */
> -/* Copyright Ingo Molnar, */
> -/* Copyright Arjan van de Ven, */
> -/* Copyright (c) International Business Machines Corp., 2008 */
> -/* */
> -/* This program is free software; you can redistribute it and/or modify */
> -/* it under the terms of the GNU General Public License as published by */
> -/* the Free Software Foundation; either version 2 of the License, or */
> -/* (at your option) any later version. */
> -/* */
> -/* This program 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 General Public License for more details. */
> -/* */
> -/* You should have received a copy of the GNU General Public License */
> -/* along with this program; if not, write to the Free Software */
> -/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */
> -/* */
> -/******************************************************************************/
> -
> -/******************************************************************************/
> -/* */
> -/* File: hackbench.c */
> -/* */
> -/* Description: hackbench tests the Linux scheduler. Test groups of 20 */
> -/* processes spraying to 20 receivers */
> -/* */
> -/* Total Tests: 1 */
> -/* */
> -/* Test Name: hackbench01 and hackbench02 */
> -/* */
> -/* Test Assertion: */
> -/* */
> -/* Author(s): Rusty Russell <rusty@rustcorp.com.au>, */
> -/* Pierre Peiffer <pierre.peiffer@bull.net>, */
> -/* Ingo Molnar <mingo@elte.hu>, */
> -/* Arjan van de Ven <arjan@infradead.org>, */
> -/* "Zhang, Yanmin" <yanmin_zhang@linux.intel.com>, */
> -/* Nathan Lynch <ntl@pobox.com> */
> -/* */
> -/* History: Included into LTP */
> -/* - June 26 2008 - Subrata Modak<subrata@linux.vnet.ibm.com>*/
> -/* */
> -/******************************************************************************/
> +// SPDX-License-Identifier: GPL-2.0-or-later
FYI we add SPDX only to files which has been converted into new LTP API. I'm ok
to change it, but it would require to add additional cleanup.
> +/*
> + * Copyright Rusty Russell,
> + * Copyright Pierre Peiffer
> + * Copyright Zhang, Yanmin,
> + * Copyright Ingo Molnar,
> + * Copyright Arjan van de Ven,
I know it's not your addition, but I wonder if a copyright without year even
makes sense.
> + * Copyright (c) International Business Machines Corp., 2008
> + */
> +
> +/*
> + * Description: hackbench tests the Linux scheduler. Test groups of 20
> + * processes spraying to 20 receivers
Missing dot at the end.
> + *
> + * Total Tests: 1
Useless, please remove.
> + *
> + * Test Name: hackbench01 and hackbench02
Useless, please remove (it can change anyway).
> + *
> + * Test Assertion:
Useless, please remove.
> + *
> + * Author(s): Rusty Russell <rusty@rustcorp.com.au>,
> + * Pierre Peiffer <pierre.peiffer@bull.net>,
> + * Ingo Molnar <mingo@elte.hu>,
> + * Arjan van de Ven <arjan@infradead.org>,
> + * "Zhang, Yanmin" <yanmin_zhang@linux.intel.com>,
> + * Nathan Lynch <ntl@pobox.com>
Hm, how about to add their mails in copyright section and remove this?
> + *
> + * History: Included into LTP
> + * - June 26 2008 - Subrata Modak<subrata@linux.vnet.ibm.com>
I would remove this (we have git history anyway) and move him to copyright
section.
> + */
> #include <pthread.h>
> #include <stdio.h>
> #include <stdlib.h>
> @@ -58,25 +40,26 @@
> #include <sys/time.h>
> #include <sys/poll.h>
> #include <limits.h>
> +#include <stdarg.h>
> -#define SAFE_FREE(p) { if (p) { free(p); (p)=NULL; } }
> +#define SAFE_FREE(p) { if (p) { free(p); (p) = NULL; } }
> #define DATASIZE 100
> static struct sender_context **snd_ctx_tab; /*Table for sender context pointers. */
> static struct receiver_context **rev_ctx_tab; /*Table for receiver context pointers. */
> -static int gr_num = 0; /*For group calculation */
> +static int gr_num; /*For group calculation */
> static unsigned int loops = 100;
> /*
> * 0 means thread mode and others mean process (default)
> */
> static unsigned int process_mode = 1;
> -static int use_pipes = 0;
> +static int use_pipes;
+1
> struct sender_context {
> unsigned int num_fds;
> int ready_out;
> int wakefd;
> - int out_fds[0];
> + int out_fds[];
> };
> struct receiver_context {
> @@ -86,9 +69,14 @@ struct receiver_context {
> int wakefd;
> };
> -static void barf(const char *msg)
> +static void barf(const char *fmt, ...)
> {
> - fprintf(stderr, "%s (error: %s)\n", msg, strerror(errno));
> + va_list args;
> +
> + va_start(args, fmt);
> + vfprintf(stderr, fmt, args);
> + va_end(args);
> + fprintf(stderr, " (error: %s)\n", strerror(errno));
> exit(1);
FYI in new C API we have tst_brk() and tst_res().
> }
> @@ -108,18 +96,18 @@ static void fdpair(int fds[2])
> if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) == 0)
> return;
> }
> - barf("Creating fdpair");
> + barf("Creating %s", __func__);
Adding va_*() just to post the name of the function looks useless for me.
Please drop this change.
> }
> /* Block until we're ready to go */
> static void ready(int ready_out, int wakefd)
> {
> char dummy;
> - struct pollfd pollfd = {.fd = wakefd,.events = POLLIN };
> + struct pollfd pollfd = {.fd = wakefd, .events = POLLIN };
> /* Tell them we're ready. */
> if (write(ready_out, &dummy, 1) != 1)
> - barf("CLIENT: ready write");
> + barf("CLIENT: %s write", __func__);
> /* Wait for "GO" signal */
> if (poll(&pollfd, 1, -1) != 1)
> @@ -210,7 +198,8 @@ pthread_t create_worker(void *ctx, void *(*func) (void *))
> barf("pthread_attr_setstacksize");
> #endif
> - if ((err = pthread_create(&childid, &attr, func, ctx)) != 0) {
> + err = pthread_create(&childid, &attr, func, ctx);
> + if (err != 0) {
> fprintf(stderr, "pthread_create failed: %s (%d)\n",
> strerror(err), err);
> exit(-1);
> @@ -235,11 +224,12 @@ void reap_worker(pthread_t id)
> }
> /* One group of senders and receivers */
> -static unsigned int group(pthread_t * pth,
> +static unsigned int group(pthread_t *pth,
> unsigned int num_fds, int ready_out, int wakefd)
> {
> unsigned int i;
> struct sender_context *snd_ctx = malloc(sizeof(struct sender_context) + num_fds * sizeof(int));
> +
> if (!snd_ctx)
> barf("malloc()");
> else
> @@ -305,8 +295,11 @@ int main(int argc, char *argv[])
> argv++;
> }
> - if (argc >= 2 && (num_groups = atoi(argv[1])) == 0)
> - print_usage_exit();
> + if (argc >= 2) {
> + num_groups = atoi(argv[1]);
> + if (num_groups == 0)
> + print_usage_exit();
> + }
+1, assigning variables is really better out of if / while,
although here it's safe, because num_groups is initialized to 10.
Kind regards,
Petr
> printf("Running with %d*40 (== %d) tasks.\n",
> num_groups, num_groups * 40);
> @@ -329,7 +322,7 @@ int main(int argc, char *argv[])
> snd_ctx_tab = malloc(num_groups * sizeof(void *));
> rev_ctx_tab = malloc(num_groups * num_fds * sizeof(void *));
> if (!pth_tab || !snd_ctx_tab || !rev_ctx_tab)
> - barf("main:malloc()");
> + barf("%s:malloc()", __func__);
> fdpair(readyfds);
> fdpair(wakefds);
> @@ -363,9 +356,8 @@ int main(int argc, char *argv[])
> /* free the memory */
> for (i = 0; i < num_groups; i++) {
> - for (j = 0; j < num_fds; j++) {
> + for (j = 0; j < num_fds; j++)
> SAFE_FREE(rev_ctx_tab[i * num_fds + j])
> - }
+1
> SAFE_FREE(snd_ctx_tab[i]);
> }
> SAFE_FREE(pth_tab);
> @@ -373,3 +365,4 @@ int main(int argc, char *argv[])
> SAFE_FREE(rev_ctx_tab);
> exit(0);
> }
> +
More information about the ltp
mailing list