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

[Ping Fang <pifang@redhat.com>]

On Tue, Mar 21, 2023 at 2:56 PM Li Wang <liwang@redhat.com> wrote:

> 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)?
>


The strtoul will try to convert string into unsigned long integer.
If successful the endptr will point to a "\0" string return the number,
otherwise to the original string and return 0.
In the case of /sys/devices/system/node/node1/cpulist, endptr is  "list\0"
and return 0. The strcmp false skip to the next iteration.


>
>
>>                                 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
>


-- 
*--*
Thanks & Regards,
*Ping Fang*

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp