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

[Ben Pfaff]

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.

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?

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 <blp at ovn.org> 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 <shu.shen at gmail.com>
> >> ---
> >>
> >> 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.