Re: [Cake] [PATCH 1/2] cake: print_uint format fixes

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 12 Mar 2018 10:56:09 +0100
Toke Høiland-Jørgensen <toke@toke.dk> wrote:

> Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> writes:
> 
> >> On 11 Mar 2018, at 23:34, Stephen Hemminger <stephen@networkplumber.org> wrote:
> >> 
> >> On Sun, 11 Mar 2018 22:10:51 +0000
> >> Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk> wrote:
> >>   
> >>> negative?
> >>> 
> >>> Tried it, and you’d sort of guess wrong. I’ll write it up tomorrow
> >>> ‘properly’ but ‘int’ is int length whereas uint is uint64 length. On
> >>> big endian it goes wrong.
> >>> 
> >>> Anyway, glad you’ve tested on something little endian. I’ll try to
> >>> submit a patch upstream as requested…very busy over next 3 days
> >>> doing $dayjob so may take a little while. Thanks for boosting
> >>> confidence that I’ve not broken it on architectures it used to work
> >>> on :-)  
> >> 
> >> print_uint should be unsigned only.
> >> 
> >> When printing json your version won't work with negative values.  
> >
> > Yes, it should be: print_int(PRINT_ANY, "overhead", "overhead %d ",
> > overhead); - certainly that works on my 32 bit big endian box.  
> 
> Yeah, this is fixed in the tc-adv git repo now :)
> 
> > Using the ‘PRId64’ macro won’t work because print_int is using ‘int’
> > type internally whereas print_uint uses ‘uint64_t’ internally. So the
> > format string has to have knowledge of the internal format, *but*
> > there’s no clue of the difference in internal format offered by the
> > function name i.e. print_int vs print_uint.
> >
> > I’d argue it makes more sense to have: print_int/print_uint as the
> > native int length, that hopefully match up with %u & %d and then have
> > print_int64/print_uint64 where use of formats PRId64 & PRIu64 is
> > advised.  
> 
> Yes, this was basically what I meant by "grating"; I really do agree
> that this API is confusing.
> 
> Stephen, would you accept patches to fix the API (to add
> print_{u,}int64() variants and turn print_uint() into native-int size)?
> Or should we stick with the API currently there and live with the
> inconsistency? :)
> 
> -Toke

I agree print_int should take int, print_uint should take unsigned int, and there should
be print_u64 (and print_u32, print_u8)