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

[Li Wang]

On Tue, Mar 21, 2023 at 7:27 PM Ping Fang <pifang@redhat.com> wrote:

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

Ah, you're right. Here we're expecting to get a valid cpuid, the
'dn->d_name + 3' should be strictly equal to "N\0".

Thanks for the explanation.

Reviewed-by: Li Wang <liwang@redhat.com>


-- 
Regards,
Li Wang