On Tue, Jan 31, 2017 at 10:45:31AM -0800, Ben Pfaff wrote: > By my count, vlan_tci_to_pcp() is used in printf-like format > specifiers in 7 places in the tree. Before this patch, it is used > with the following format specifiers: > > %d 3 times %x 1 time PRIu8 1 time PRIx8 1 time > > Both %d and %x are obviously correct, portable format specifiers for > int, which is the return type of vlan_tci_to_pcp(). I contend that > PRIu8 and PRIx8 should be acceptable too because the integer > promotions convert uint8_t to int anyway. The problem is that PRIu8 and PRIx8 the specifiers are not promoted to 'u' and 'x' respectively on macOS.
For example, for PRIu*, on Mac OS, /usr/include/inttypes.h: # define __PRI_8_LENGTH_MODIFIER__ "hh" # define __PRI_64_LENGTH_MODIFIER__ "ll" # define PRIu8 __PRI_8_LENGTH_MODIFIER__ "u" # define PRIu16 "hu" # define PRIu32 "lu" # define PRIu64 __PRI_64_LENGTH_MODIFIER__ "u" While on Linux/glibc, /usr/include/inttypes.h: # define PRIu8 "u" # define PRIu16 "u" # define PRIu32 "u" # define PRIu64 __PRI64_PREFIX "u" where all PRIu* except PRIu64 are the same. Therefore, using PRIu8 or PRIx8 with the int return type of vlan_tci_to_pcp() is causing compiler warnings on macOS. As PRIu8 and PRIx8 are indeed accurate specifier for pcp (Priority code point), the patch proposes changing the return type of vlan_tci_to_pcp(). In addition, the patch includes changes of format specifiers to consistently use PRI* specifier. But unfortunately as you pointed out below, there are two "%d" occurrences I missed. I'll put these specifier changes into a separate commit for clarity. > > After this patch, vlan_tci_to_pcp() returns uint8_t and it is used in > the following format specifiers: > > %d 2 times > PRIu8 2 times > PRIx8 2 times > > I still don't see the point. You're saying, effectively, that %x is not > acceptable but all the others are. How is that possible? Please see above, the intention is to use PRI* specifier throughout, I missed the two occurrences of '%d'. Will fix in next revision of the patch(set). > > On Sat, Jan 14, 2017 at 09:59:33AM -0800, Shu Shen wrote: > > The main change of this patch is in lib/packets.h, where the return type of > > vlan_tci_to_pcp() and vlan_tci_to_cfi() are changed from int to uint8_t. > > > > Changes to the format specifiers are for portability to mac OS and > > consistency use of PRIu* specifiers. > > > > On Sat, Jan 14, 2017 at 8:47 AM, Ben Pfaff <[email protected]> wrote: > > > On Fri, Jan 13, 2017 at 02:04:38PM -0800, Shu Shen wrote: > > >> The return type of vlan_tci_to_pcp() was int where it's expected to be > > >> uint8_t and causing implicit truncation when the function is used. On > > >> some platforms such as macOS, where PRIu8 is defined as "hhx" and no > > >> promotion of short to int is done, the compiler might throw out Wformat > > >> message for ds_put_format() calls on the returns value of > > >> vlan_tci_to_pcp(). > > >> > > >> vlan_tci_to_cfi() is also fixed with uint8_t as return type although the > > >> function is not currently being used anywhere. > > >> > > >> Format strings in ds_put_format() for printing out returned values from > > >> vlan_tci_to_pcp() were updated to ensure PRIu8 or PRIx8 are used for > > >> portability. > > >> > > >> Signed-off-by: Shu Shen <[email protected]> > > >> --- > > >> > > >> v2: Fixed typoes for uint8_t in commit message > > > > > > This one doesn't really make sense to me. This patch only changes two > > > format specifiers away from %d or %x, which are both correct format > > > specifiers for "int". Can you explain? > > > > > > Thanks, > > > > > > Ben. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
