[LTP] [PATCH] getcpu01: Fix strtoul incorrectly returns 0

[Li Wang]

Hi Ping,

Good catch!

On Mon, Mar 20, 2023 at 5:23 PM Ping Fang <pifang@redhat.com> wrote:

> get_nodeid() find nodeid by iterate /sys/devices/system/node/nodeX/cpuY.
> But there are cpulist and cpumap, which will report 0 in strtoul.
> On multi-node numa system, the last iteration nodeX directory mismatch
> the cpu0 node id. Then fail the test. The following shows on a 4 node
> numa system. The last iteration stopped under /node1. so expected node
> id 1. Then failed.
>
> getcpu01.c:98: TINFO: cpu:0, cpu_id:0, de->d_name:node2
> getcpu01.c:99: TINFO: dn->d_name:cpulist
> getcpu01.c:98: TINFO: cpu:0, cpu_id:0, de->d_name:node0
> getcpu01.c:99: TINFO: dn->d_name:cpulist
> getcpu01.c:98: TINFO: cpu:0, cpu_id:0, de->d_name:node3
> getcpu01.c:99: TINFO: dn->d_name:cpulist
> getcpu01.c:98: TINFO: cpu:0, cpu_id:0, de->d_name:node1
> getcpu01.c:99: TINFO: dn->d_name:cpulist
> getcpu01.c:128: TFAIL: getcpu() returned wrong value expected  node id:1
> returned  node id:0, cpu_set:0
>
> Reported-by: Shizhao Chen <shichen@redhat.com>
> Signed-off-by: Ping Fang <pifang@redhat.com>
> ---
>  testcases/kernel/syscalls/getcpu/getcpu01.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/getcpu/getcpu01.c
> b/testcases/kernel/syscalls/getcpu/getcpu01.c
> index f6fcc4fc1..9842c8999 100644
> --- a/testcases/kernel/syscalls/getcpu/getcpu01.c
> +++ b/testcases/kernel/syscalls/getcpu/getcpu01.c
> @@ -69,6 +69,7 @@ static unsigned int get_nodeid(unsigned int cpu_id)
>         DIR *directory_parent, *directory_node;
>         struct dirent *de, *dn;
>         char directory_path[PATH_MAX];
> +       char *invalid_number;
>         unsigned int cpu;
>         int node_id = 0;
>
> @@ -91,7 +92,9 @@ static unsigned int get_nodeid(unsigned int cpu_id)
>                         while ((dn = readdir(directory_node)) != NULL) {
>                                 if (strncmp(dn->d_name, "cpu", 3))
>                                         continue;
> -                               cpu = strtoul(dn->d_name + 3, NULL, 0);
> +                               cpu = strtoul(dn->d_name + 3,
> &invalid_number, 0);
> +                               if (strcmp(invalid_number, "\0"))
> +                                       continue;
>

Why only check if **endptr is '\0' on return? Shouldn't we consider
a more generic situation that was no digits and *endptr stores the
original string (or returns 0)?



>                                 if (cpu == cpu_id) {
>                                         node_id =
>                                             strtoul(de->d_name + 4, NULL,
> 0);
> --
> 2.31.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang