<div><div dir="auto">Nice work Kevin.</div><br><div class="gmail_quote"><div>On Sat, Mar 10, 2018 at 09:56 Kevin Darbyshire-Bryant <<a href="mailto:kevin@darbyshire-bryant.me.uk">kevin@darbyshire-bryant.me.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
> On 8 Mar 2018, at 11:21, Toke Høiland-Jørgensen <<a href="mailto:toke@toke.dk" target="_blank">toke@toke.dk</a>> wrote:<br>
><br>
>><br>
>> Oh and curiously the bad values go away if you ask for json output<br>
>> it’s much better. Which rather points at a ‘feature’ of the<br>
>> ‘print_string’ behaviour.<br>
><br>
> Right. Well, the print_* functions are behind several levels of<br>
> pre-processor indirection, so not quite obvious what's going on here.<br>
> Don't really see why they should spit out garbage values, though.<br>
><br>
><br>
> Stephen, do you have any ideas?<br>
><br>
> -Toke<br>
<br>
Right, cracked it and it’s horrible!<br>
<br>
print_uint is expanded thus: Note the type of value uint64_t<br>
<br>
void print_color_uint(enum output_type t, enum color_attr color, const char *key, const char *fmt, uint64_t value);<br>
static inline void print_uint (enum output_type t, const char *key, const char *fmt, uint64_t value)<br>
{ print_color_uint( t, COLOR_NONE, key, fmt, value); };<br>
<br>
So far so good.<br>
<br>
print_color_uint expands to:<br>
<br>
void print_color_uint(enum output_type t, enum color_attr color, const char *key, const char *fmt, uint64_t value)<br>
{<br>
if (((t & PRINT_JSON || t & PRINT_ANY) && _jw))<br>
{ if (!key) jsonw_uint(_jw, value);<br>
else jsonw_uint_field(_jw, key, value);<br>
}<br>
else if ((!_jw && (t & PRINT_FP || t & PRINT_ANY)))<br>
{ color_fprintf( (stdout) , color, fmt, value);<br>
}<br>
};<br>
<br>
Again, no issue and we eventually call color_fprintf<br>
<br>
int color_fprintf(FILE *fp, enum color_attr attr, const char *fmt, ...)<br>
{<br>
int ret = 0;<br>
va_list args;<br>
<br>
va_start(args, fmt);<br>
<br>
if (!color_is_enabled || attr == COLOR_NONE) {<br>
ret = vfprintf(fp, fmt, args);<br>
goto end;<br>
}<br>
<br>
<br>
Now, color_printf is a variable argument list function and as such is dependent upon being told the correct size of argument variables in the fmt variable. And that’s our problem, we’re passing a 64bit integer but telling the format string that it’s ‘int’…which I’m guessing can be variable sizes depending on architecture, as can the endianness.<br>
<br>
If we instead do (in q_cake.c)<br>
<br>
#include <inttypes.h><br>
<br>
print_uint(PRINT_ANY, "min_transport_size", " min/max transport layer size: %10" PRIu64, stnc->min_trnlen);<br>
<br>
it works. This needs sanity checking by a clever person.<br>
<br>
Cheers,<br>
<br>
Kevin D-B<br>
<br>
012C ACB2 28C6 C53E 9775 9123 B3A2 389B 9DE2 334A<br>
<br>
_______________________________________________<br>
Cake mailing list<br>
<a href="mailto:Cake@lists.bufferbloat.net" target="_blank">Cake@lists.bufferbloat.net</a><br>
<a href="https://lists.bufferbloat.net/listinfo/cake" rel="noreferrer" target="_blank">https://lists.bufferbloat.net/listinfo/cake</a><br>
</blockquote></div></div>