[ovs-dev] [PATCH v2] packet: proper return type for vlan_tci_to_pcp()
Ben Pfaff
blp at ovn.org
Thu Feb 2 23:08:09 UTC 2017
On Thu, Feb 02, 2017 at 02:55:03PM -0800, Shu Shen wrote:
> 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.
What are the warnings?
There is no real problem with using %hhd or %hhu to print an int,
because printf() cannot tell the difference between an int and a char:
they are both passed as int.
> 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.
I don't see how putting them in a separate commit provides "clarity"
since the lack of the changes is most of my problem with the commit.
More information about the dev
mailing list