[CRIU] [PATCH 5/5] compel std: rename printing functions

Dmitry Safonov dsafonov at virtuozzo.com
Tue Feb 14 01:47:27 PST 2017


On 02/14/2017 03:59 AM, Kir Kolyshkin wrote:
> Let's rename the printing functions so their names look more like
> the standard ones.
>
> 1. putc/puts with a file descriptor.
>
> __std_putc -> std_dputc
> __std_puts -> std_dputs
>
> There are no standard putc/puts that accept fd as an argument,
> but the libc convention is to use d prefix for such. Therefore:
>
> NOTE we keep the order of the arguments intact, to be in line
> with the rest of the functions.
>
> 2. *printf
>
> __std_printk -> std_vdprintf
> __std_printf -> std_dprintf
>
> The reason is, these are the names of libc functions with similar
> functionality/arguments.
>
> Cc: Dmitry Safonov <dsafonov at virtuozzo.com>
> Cc: Cyrill Gorcunov <gorcunov at openvz.org>
> Signed-off-by: Kir Kolyshkin <kir at openvz.org>

Yep, I found it at least strange, at worst misleading that common
pattern in kernel/libc:
int printf/printk(char *fmt, ...)
{
// ...
va_start()
ret = vprinf()/vprintk()
va_end()
return ret;
}

Was not here.
It's quite handy to have v-prefixed version when you need to pass
va_arg to it (simple example - declaring a wrapper).
So, it's not convenient that printf() calls printk(), which from
a glance looks more  like a function that opens /dev/kmsg and writes
into it.

FWIW,
Reviewed-by: Dmitry Safonov <dsafonov at virtuozzo.com>

> ---
>  compel/plugins/include/uapi/std/string.h | 15 +++++++--------
>  compel/plugins/std/string.c              | 20 ++++++++++----------
>  2 files changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/compel/plugins/include/uapi/std/string.h b/compel/plugins/include/uapi/std/string.h
> index 5129017..c2e4b93 100644
> --- a/compel/plugins/include/uapi/std/string.h
> +++ b/compel/plugins/include/uapi/std/string.h
> @@ -11,16 +11,15 @@
>  #define	STDERR_FILENO	2	/* Standard error output.  */
>
>
> -extern void __std_putc(int fd, char c);
> -extern void __std_puts(int fd, const char *s);
> -extern void __std_printk(int fd, const char *format, va_list args);
> -extern void __std_printf(int fd, const char *format, ...)
> +extern void std_dputc(int fd, char c);
> +extern void std_dputs(int fd, const char *s);
> +extern void std_vdprintf(int fd, const char *format, va_list args);
> +extern void std_dprintf(int fd, const char *format, ...)
>  	__attribute__ ((__format__ (__printf__, 2, 3)));
>
> -
> -#define std_printf(fmt, ...)	__std_printf(STDOUT_FILENO, fmt, ##__VA_ARGS__)
> -#define std_puts(s)		__std_puts(STDOUT_FILENO, s)
> -#define std_putchar(c)		__std_putc(STDOUT_FILENO, c)
> +#define std_printf(fmt, ...)	std_dprintf(STDOUT_FILENO, fmt, ##__VA_ARGS__)
> +#define std_puts(s)		std_dputs(STDOUT_FILENO, s)
> +#define std_putchar(c)		std_dputc(STDOUT_FILENO, c)
>
>  extern unsigned long std_strtoul(const char *nptr, char **endptr, int base);
>  extern int std_strcmp(const char *cs, const char *ct);
> diff --git a/compel/plugins/std/string.c b/compel/plugins/std/string.c
> index 177d7e6..43db1e3 100644
> --- a/compel/plugins/std/string.c
> +++ b/compel/plugins/std/string.c
> @@ -9,15 +9,15 @@
>
>  static const char conv_tab[] = "0123456789abcdefghijklmnopqrstuvwxyz";
>
> -void __std_putc(int fd, char c)
> +void std_dputc(int fd, char c)
>  {
>  	sys_write(fd, &c, 1);
>  }
>
> -void __std_puts(int fd, const char *s)
> +void std_dputs(int fd, const char *s)
>  {
>  	for (; *s; s++)
> -		__std_putc(fd, *s);
> +		std_dputc(fd, *s);
>  }
>
>  static size_t __std_vprint_long_hex(char *buf, size_t blen, unsigned long num, char **ps)
> @@ -74,7 +74,7 @@ done:
>  	return blen - (s - buf);
>  }
>
> -void __std_printk(int fd, const char *format, va_list args)
> +void std_vdprintf(int fd, const char *format, va_list args)
>  {
>  	const char *s = format;
>
> @@ -83,7 +83,7 @@ void __std_printk(int fd, const char *format, va_list args)
>  		int along = 0;
>
>  		if (*s != '%') {
> -			__std_putc(fd, *s);
> +			std_dputc(fd, *s);
>  			continue;
>  		}
>
> @@ -97,7 +97,7 @@ void __std_printk(int fd, const char *format, va_list args)
>
>  		switch (*s) {
>  		case 's':
> -			__std_puts(fd, va_arg(args, char *));
> +			std_dputs(fd, va_arg(args, char *));
>  			break;
>  		case 'd':
>  			__std_vprint_long(buf, sizeof(buf),
> @@ -105,7 +105,7 @@ void __std_printk(int fd, const char *format, va_list args)
>  					  va_arg(args, long) :
>  					  (long)va_arg(args, int),
>  					  &t);
> -			__std_puts(fd, t);
> +			std_dputs(fd, t);
>  			break;
>  		case 'x':
>  			__std_vprint_long_hex(buf, sizeof(buf),
> @@ -113,18 +113,18 @@ void __std_printk(int fd, const char *format, va_list args)
>  					      va_arg(args, long) :
>  					      (long)va_arg(args, int),
>  					      &t);
> -			__std_puts(fd, t);
> +			std_dputs(fd, t);
>  			break;
>  		}
>  	}
>  }
>
> -void __std_printf(int fd, const char *format, ...)
> +void std_dprintf(int fd, const char *format, ...)
>  {
>  	va_list args;
>
>  	va_start(args, format);
> -	__std_printk(fd, format, args);
> +	std_vdprintf(fd, format, args);
>  	va_end(args);
>  }
>
>


-- 
              Dmitry


More information about the CRIU mailing list