Re: [PATCHv3 2/3] lib: printf: append support of '%*ph[CDN]'

[Joe Perches <joe@perches.com>]

On Wed, 2012-07-04 at 11:45 +0300, Andy Shevchenko wrote:
> This patch adds a support of the variable width buffer to print it
> as a hex string with a delimiter.

Hi again Andy.

> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
[]
> @@ -655,6 +655,57 @@ char *resource_string(char *buf, char *end, struct resource *res,
>  }
>  
>  static noinline_for_stack
> +char *hex_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
> +		 const char *fmt)
> +{
> +	char hex_str[8*3+1];	/* support up to 8 bytes to print */

I think you don't need hex_str at all.

[]

> +	if (spec.field_width <= 0)
> +		/* nothing to print */
> +		return buf;

It may be better to default to a 1 and add

	if (addr == ZERO_OR_NULL_PTR)

to avoid dereferencing a NULL or a pointer
to a zero length object.

> +
> +	len = min_t(int, spec.field_width, 64);
> +
> +	while (i < len) {
> +		p = hex_str;
> +		for (j = 0; j < 8 && i < len; j++, i++) {
> +			p = hex_byte_pack(p, addr[i]);
> +
> +			if (separator && i != len - 1)
> +				*p++ = separator;
> +		}
> +		*p = '\0';
> +
> +		for (p = hex_str; *p != '\0'; p++) {
> +			if (buf < end)
> +				*buf = *p;
> +			++buf;
> +		}

why not just directly write to *buf as long as buf < end?

cheers, Joe