Re: [PATCH for-4.5 1/2] xen/vsprintf: Introduce %*ph extended format specifier for hex buffers

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 26/09/14 13:31, Jan Beulich wrote:
>>>> On 26.09.14 at 14:16, <andrew.cooper3@citrix.com> wrote:
>> On 26/09/14 12:32, Jan Beulich wrote:
>>>>>> On 26.09.14 at 12:10, <andrew.cooper3@citrix.com> wrote:
>>>> --- a/docs/misc/printk-formats.txt
>>>> +++ b/docs/misc/printk-formats.txt
>>>> @@ -18,3 +18,9 @@ Symbol/Function pointers:
>>>>  
>>>>         %pv     Domain and vCPU ID from a 'struct vcpu *' (printed as
>>>>                 "d<domid>v<vcpuid>")
>>>> +
>>>> +
>>>> +Raw buffer as hex string:
>>>> +
>>>> +       %*ph    Up to 64 characters, printed as "00 01 02 ... ff".  Buffer 
>> length
>>>> +               expected via the field_width paramter. i.e. printk("%*ph", 
>> 8, buffer);
>>> Let's keep this list sorted alphabetically please.
>> Ok, but then the "Symbol/Function pointers:" paragraph marker should be
>> dropped.
>>
>> I am happy with doing either.
> Actually it looks like I should have added a header when adding %pv,
> so maybe that's what wants to be corrected? Sorting by formatting
> character still would see desirable to me, as would keeping the
> headings.

I will introduce the heading for %pv

>
>>>> --- a/xen/common/vsprintf.c
>>>> +++ b/xen/common/vsprintf.c
>>>> @@ -272,6 +272,31 @@ static char *pointer(char *str, char *end, const char **fmt_ptr,
>>>>      /* Custom %p suffixes. See XEN_ROOT/docs/misc/printk-formats.txt */
>>>>      switch ( fmt[1] )
>>>>      {
>>>> +    case 'h': /* Raw buffer as hex string. */
>>>> +    {
>>>> +        /*
>>>> +         * User expected to provide an explicit count using %*.  Bound between
>>>> +         * 0 and 64 bytes, defaulting to 0.
>>>> +         */
>>>> +        unsigned i, nr_bytes =
>>>> +            ((field_width < 1) || (field_width > 64)) ? 0 : field_width;
>>> Producing no output for too small a field width makes sense, but why
>>> not print 64 bytes if more were requested?
>> 64 is arbitrary (taken from the Linux statement to the same effect). 
>> Even with an upper bound of 64, the caller should be using something
>> shorter and putting in newlines.
> I'd be fine with you limiting it to a lower value; I just find it odd to
> zap a value exceeding the boundary to zero rather than to the
> upper bound.
>
> Jan
>

Ok

~Andrew