[ovs-dev] [PATCH v2] packet: proper return type for vlan_tci_to_pcp()

Shu Shen shu.shen at gmail.com
Thu Feb 2 23:31:03 UTC 2017


On Thu, Feb 02, 2017 at 03:08:09PM -0800, Ben Pfaff wrote:
> 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?
See https://travis-ci.org/openvswitch/ovs/jobs/197378752#L1303
There are plenty of other warnings due to similar situation (if they are
not problems).

> 
> 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.
You may be right about printf with regard to an int and a char. But how
about an int specifier and a long variable?

When I noticed the Wformat warning messages on macOS, I have no idea
whether there is a hidden issue where the format specifier might be
truncating a variable.

The only way to be sure is to clean up the code base to remove the
warning messages even if they are just noise.

> 
> > 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