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.