Hi,
On 02/09/2015 02:12 PM, Liang, Cunming wrote:
>>> +int
>>> +rte_thread_get_affinity(rte_cpuset_t *cpusetp)
>>> +{
>>> + if (!cpusetp)
>>> + return -1;
>>
>> Same here. This is the only reason why rte_thread_get_affinity() could
>> fail. Removing this test would allow to change the API to return void
>> instead. It will avoid a useless test below in
>> eal_thread_dump_affinity().
> [LCM] The cpusetp is used as destination of memcpy and the function suppose
> an EAL API.
> I don't think it's a good idea to remove the check, do you ?
I know we often have debate on this subject on the list. My personal
opinion is that checking a NULL pointer in these cases is useless
because the user is suppose to give a non-NULL pointer. Returning
an error will result in managing an error for something that cannot
happen.
On the other hand, adding an assert() (or the dpdk equivalent) would
be a good idea.
>>
>>> +
>>> + rte_memcpy(cpusetp, &RTE_PER_LCORE(_cpuset),
>>> + sizeof(rte_cpuset_t));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +void
>>> +eal_thread_dump_affinity(char str[], unsigned size)
>>> +{
>>> + rte_cpuset_t cpuset;
>>> + unsigned cpu;
>>> + int ret;
>>> + unsigned int out = 0;
>>> +
>>> + if (rte_thread_get_affinity(&cpuset) < 0) {
>>> + str[0] = '\0';
>>> + return;
>>> + }
>>
>> This one could be removed it the (== NULL) test is removed.
>>
>>> +
>>> + for (cpu = 0; cpu < RTE_MAX_LCORE; cpu++) {
>>> + if (!CPU_ISSET(cpu, &cpuset))
>>> + continue;
>>> +
>>> + ret = snprintf(str + out,
>>> + size - out, "%u,", cpu);
>>> + if (ret < 0 || (unsigned)ret >= size - out)
>>> + break;
>>
>> On the contrary, I think here returning an error to the user
>> would be useful so he can knows that the dump is not complete.
> [LCM] accept.
>>
>>
>> Regards,
>> Olivier
