Re: [PATCH v3 4/5] telemetry: rename local variables

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Wed, Apr 05, 2023 at 05:03:25PM +0100, Bruce Richardson wrote:
> In the newly separated out function, rename "tmp" to "buf" to have more
> meaningful variable names.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> ---
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

(with suggestions)

> 
> When committing, this patch can be merged with the previous. I've kept
> them separate for now, as it makes reviewing a lot easier.
> ---
>  lib/telemetry/telemetry_json.h | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h
> index aada523a27..c087b833eb 100644
> --- a/lib/telemetry/telemetry_json.h
> +++ b/lib/telemetry/telemetry_json.h
> @@ -84,44 +84,44 @@ static const char control_chars[0x20] = {
>   * directly, but returns 0 on overflow. Otherwise returns number of chars written to buffer.
>   */
>  static inline int
> -__json_format_str_to_buf(char *tmp, const int len,
> +__json_format_str_to_buf(char *buf, const int len,
>  		const char *prefix, const char *str, const char *suffix)

does it cascade rubbish into the caller if `len` is made to be type
`size_t` instead of `int`?

>  {
> -	int tmpidx = 0;
> +	int bufidx = 0;
>  
> -	while (*prefix != '\0' && tmpidx < len)
> -		tmp[tmpidx++] = *prefix++;
> -	if (tmpidx >= len)
> +	while (*prefix != '\0' && bufidx < len)
> +		buf[bufidx++] = *prefix++;
> +	if (bufidx >= len)
>  		return 0;
>  
>  	while (*str != '\0') {
>  		if (*str < (int)RTE_DIM(control_chars)) {
>  			int idx = *str;  /* compilers don't like char type as index */

should be `size_t` instead of `int` type for idx.

>  			if (control_chars[idx] != 0) {
> -				tmp[tmpidx++] = '\\';
> -				tmp[tmpidx++] = control_chars[idx];
> +				buf[bufidx++] = '\\';
> +				buf[bufidx++] = control_chars[idx];
>  			}
>  		} else if (*str == '"' || *str == '\\') {
> -			tmp[tmpidx++] = '\\';
> -			tmp[tmpidx++] = *str;
> +			buf[bufidx++] = '\\';
> +			buf[bufidx++] = *str;
>  		} else
> -			tmp[tmpidx++] = *str;
> +			buf[bufidx++] = *str;
>  		/* we always need space for (at minimum) closing quote and null character.
>  		 * Ensuring at least two free characters also means we can always take an
>  		 * escaped character like "\n" without overflowing
>  		 */
> -		if (tmpidx > len - 2)
> +		if (bufidx > len - 2)
>  			return 0;
>  		str++;
>  	}
>  
> -	while (*suffix != '\0' && tmpidx < len)
> -		tmp[tmpidx++] = *suffix++;
> -	if (tmpidx >= len)
> +	while (*suffix != '\0' && bufidx < len)
> +		buf[bufidx++] = *suffix++;
> +	if (bufidx >= len)
>  		return 0;
>  
> -	tmp[tmpidx] = '\0';
> -	return tmpidx;
> +	buf[bufidx] = '\0';
> +	return bufidx;
>  }
>  
>  /**
> -- 
> 2.37.2