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

[Bruce Richardson <bruce.richardson@intel.com>]

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