On 6/13/2024 8:13 PM, Stephen Hemminger wrote:
> On Thu, 13 Jun 2024 17:51:14 +0100
> Ferruh Yigit <ferruh.yi...@amd.com> wrote:
> 
>>> Hi Sivaprasad,
>>>
>>> Is this '(lcoreid_t)' cast required? Because of integer promotion I
>>> think result will be correct without casting.
>>>
>>> (And without integer promotion considered, casting needs to be done on
>>> one of the variables, not to the result, because result may be already
>>> cast down I think. Anyway this is not required for this case since
>>> variables are u16.)
>>>  
>>
>> Why casing required (for record) is,
>> 'nb_txq' -> uint16_t, promoted to 'int'
>> 'nb_fwd_ports' -> uint16_t, promoted to 'int'
>> (nb_txq * nb_fwd_ports) -> result 'int'
>> nb_fwd_lcores  -> 'uint32_t'
>>
>> comparison between 'int' & 'uint32_t' gives warning. After some compiler
>> version it is smart enough to not give a warning, but casting is
>> required for old compilers.
>>
>> And back to my comment above, casting one of the parameter to
>> 'lcoreid_t' also works, as it forcing promotion to 'unsigned int'
>> instead. But logically casting looks odd, so keeping casting result.
> 
> Where is the integer promotion happening?
>

Raslan reported following compile error, this version of the patch has
the cast, but merged version, v3, doesn't.


```
../../root/dpdk/app/test-pmd/config.c: In function 'icmp_echo_config_setup':
../../root/dpdk/app/test-pmd/config.c:5159:30: error: comparison between
signed and unsigned integer expressions [-Werror=sign-compare]
  if ((nb_txq * nb_fwd_ports) < nb_fwd_lcores)
                              ^
```

> Also would be better to use strtoul() instead of atoi() when parsing the args.
> That gives better error handling and handles someone passing negative number 
> correctly.

Reply via email to