[v3,4/5] telemetry: rename local variables

Message ID 20230405160326.186921-5-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series telemetry: remove variable length arrays |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson April 5, 2023, 4:03 p.m. UTC
  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>

---

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(-)
  

Comments

Tyler Retzlaff April 7, 2023, 7:50 p.m. UTC | #1
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
  
Bruce Richardson April 11, 2023, 8:58 a.m. UTC | #2
On Fri, Apr 07, 2023 at 12:50:06PM -0700, Tyler Retzlaff wrote:
> 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.
> 
Agreed. However, trying to keep this as a pure rename only patch, so I
don't plan on fixing that here. If I do a new version of the series, I'll
see if I can work it into the follow-on patch easily.

/Bruce
  

Patch

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)
 {
-	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 */
 			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;
 }
 
 /**