Re: [Qemu-devel] [PATCH] hmp: avoid redundant null termination of buffer

[Wolfgang Bumiller]

> On January 12, 2016 at 5:00 PM Markus Armbruster <address@hidden> wrote:
>         separator = strchr(keys, '-');
>         keyname_len = separator ? separator - keys : strlen(keys);
>         pstrcpy(keyname_buf, sizeof(keyname_buf), keys);
>         [...]
>         keyname_buf[keyname_len] = 0;
> 
> This is stupid.  If we already know how many characters we need, we
> should copy exactly those:

I mentioned that and didn't touch it because the same holds for the
copying of the word "less" below and should IMO be in a separate
cleanup patch together with that one.

>         separator = strchr(keys, '-');
>         keyname_len = separator ? separator - keys : strlen(keys);
>         if (keyname_len >= sizeof(keyname_buf))
>             goto err_out;
>         memcpy(keyname_buf, keyname_len, keys);
>         keyname_buf[keyname_len] = 0;
> 
> But I'd simply dispense with the static buffer, and do something like
> 
>         separator = strchr(keys, '-');
>         key = separator ? g_strndup(keys, separator - keys) : g_strdup(keys);
>         [...]
>         g_free(key);
> 
> This is advice, not recommendation.

Sure, also a good alternative.

> (...)
> 
> Will you prepare a revised patch?

Can do that tomorrow, but which option is the preferred one? If "%.*s" works
everywhere then changing index_from_key() and using "%.*s" would be the most
optimal I think.

I don't want to bounce 5 more versions back and forth of something that's
supposed to be rather trivial.