Re: [LTP] [PATCH] Create a function tst_resm_hexd to print a buffer in hex

[<ch...@su...>]

A little more detailed description of the new function here please.

> Signed-off-by: Alexey Kodanev <ale...@or...>
> ---
> @@ -40,6 +40,7 @@
>   *    FUNCTION NAME     :
>   *      tst_res() -       Print result message (include file contents)
>   *      tst_resm() -      Print result message
> + *      tst_resm_hexd() - Print result message (add buffer contents in hex)
>   *      tst_brk() -       Print result message (include file contents)
>   *                        and break remaining test cases
>   *      tst_brkm() -      Print result message and break remaining test
> @@ -62,6 +63,12 @@
>   *      int  ttype;
>   *      char *tmesg;
>   *
> + *      void tst_resm_hexd(ttype, buf, size, tmesg [,arg]...)
> + *      int  ttype;
> + *      const void *buf;
> + *      size_t size;
> + *      char *tmesg;

This part of the comment is IMHO useless as nobody will read the C code
when looking for documentation and evenmore this does not add more
information than the prototype in header. (I made a note to clean these
up later).

>   *      void tst_brk(ttype, fname, cleanup, tmesg, [,argv]...)
>   *      int  ttype;
>   *      char *fname;
> @@ -675,6 +682,47 @@ void tst_resm(int ttype, char *arg_fmt, ...)
>  }
>  
>  /*
> + * tst_resm_hexd() - Interface to tst_res(), with no filename.
> + * Also, dump specified buffer in hex.
> + */
> +void tst_resm_hexd(int ttype, const void *buf, size_t size, char *arg_fmt, ...)
> +{
> +	char tmesg[USERMESG];
> +
> +#if DEBUG
> +	printf("IN tst_resm_hexd\n");
> +	fflush(stdout);
> +#endif
> +
> +	EXPAND_VAR_ARGS(tmesg, arg_fmt, USERMESG);
> +
> +	static const char msg_end[] = " ...";
> +	static const size_t symb_num = 3; /* space + xx */
> +	size_t end_num = strlen(msg_end);

I think that todays compilers can avoid the call to strlen() on static
string but I would either hardcode the size of the string below or even
better use sizeof() which returns strlen() + 1 in this case.

> +	size_t offset = strlen(tmesg);
> +	int size_changed = 0;
> +
> +	if ((offset + symb_num * size + 1) >= USERMESG) {
> +		size = (USERMESG - offset) / symb_num - end_num - 1;

(USERMESG - offset - sizeof(msg_end)) / symb_num ?

> +		size_changed = 1;
> +	}
> +
> +	char *pmesg = tmesg + offset;
> +
> +	size_t i;
> +	for (i = 0; i < size; ++i) {
> +		sprintf(pmesg, " %02x", ((unsigned char *)buf)[i]);
> +		pmesg += symb_num;
> +	}
> +
> +	if (size_changed)
> +		strcat(pmesg, msg_end);

Shouldn't strcpy() suffice here?

> +	tst_res(ttype, NULL, "%s", tmesg);
> +}

-- 
Cyril Hrubis
ch...@su...