[SLOF] [PATCH slof] libc: Port vsnprintf back from skiboot

[Thomas Huth]

On 01/12/15 08:14, Alexey Kardashevskiy wrote:
> Since initial port from slof to skiboot, vsnprintf() has improved in
> skiboot so let's port the improved version back.
> 
> Suggested-by: Thomas Huth <thuth at redhat.com>
> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> ---
> 
> btw the initial commit of it is a bit different from the current slof version.

Yes, looks like the initial commit in skiboot already contained support
for uppercase hex number printing which is still lacking in the current
version of SLOF.

> diff --git a/lib/libc/stdio/vsnprintf.c b/lib/libc/stdio/vsnprintf.c
> index e78fb3d..96a48b9 100644
> --- a/lib/libc/stdio/vsnprintf.c
> +++ b/lib/libc/stdio/vsnprintf.c
> @@ -10,72 +10,112 @@
>   *     IBM Corporation - initial implementation
>   *****************************************************************************/
>  
> +#include <stdbool.h>
>  #include "stdio.h"
>  #include "stdlib.h"
>  #include "string.h"
> +#include "ctype.h"
>  
>  const static unsigned long long convert[] = {

Skiboot uses "static const" instead of "const static" ... could you
maybe change that, too? ... GCC seems to complain about "const static"
when using the "-Wextra" compiler flag.

>  	0x0, 0xFF, 0xFFFF, 0xFFFFFF, 0xFFFFFFFF,
>  	0xFFFFFFFFFFULL, 0xFFFFFFFFFFFFULL, 0xFFFFFFFFFFFFFFULL, 0xFFFFFFFFFFFFFFFFULL
>  };
>  
> -
> -
>  static int
> -print_itoa(char **buffer,unsigned long value, unsigned short int base)
> +print_str_fill(char **buffer, size_t bufsize, char *sizec,
> +					const char *str, char c)
>  {
> -	const char zeichen[] = {'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F'};
> -	static char sign = 0;
> +	int i, sizei, len;
> +	char *bstart = *buffer;
>  
> -	if(base <= 2 || base > 16)
> -		return 0;
> -
> -	if(value < 0) {
> -		sign = 1;
> -		value *= -1;
> -	}
> -
> -	if(value < base) {
> -		if(sign) {
> -			**buffer = '-';
> +	sizei = strtoul(sizec, NULL, 10);
> +	len = strlen(str);
> +	if (sizei > len) {
> +		for (i = 0;
> +			(i < (sizei - len)) && ((*buffer - bstart) < bufsize);
> +									i++) {
> +			**buffer = c;
>  			*buffer += 1;
> -			sign = 0;
>  		}
> -		**buffer = zeichen[value];
> -		*buffer += 1;
> -	} else {
> -		print_itoa(buffer, value / base, base);
> -		**buffer = zeichen[(value % base)];
> -		*buffer += 1;
>  	}
> +	return 1;
> +}
>  
> +static int
> +print_str(char **buffer, size_t bufsize, const char *str)
> +{
> +	char *bstart = *buffer;
> +	int i;
> +
> +	for (i = 0; (i < strlen(str)) && ((*buffer - bstart) < bufsize); i++) {

Hmmm, looks like somebody from the skiboot folks is suffering from
paranthesitis, too ... ;-)

> +		**buffer = str[i];
> +		*buffer += 1;
> +	}
>  	return 1;
>  }
>  
>  
> +

Maybe remove that additional empty line ... skiboot does not have it either.

>  static unsigned int
>  print_intlen(unsigned long value, unsigned short int base)
>  {
>  	int i = 0;
>  
> -	while(value > 0) {
> +	while (value > 0) {
>  		value /= base;
>  		i++;
>  	}
> -	if(i == 0) i = 1;
> +	if (i == 0)
> +		i = 1;
>  	return i;
>  }

Just some cosmetic nits ... remaining part looks fine to me, so:

Reviewed-by: Thomas Huth <thuth at redhat.com>