On 15.01.20 14:56, Alberto Garcia wrote: > This is a bit more efficient than having to allocate and free memory > for each item. > > The default size (60) is enough for all the existing incompatible > features or the "Unknown incompatible feature" message.
That doesn’t make sense to me. The existing incompatible features are known to qemu, and as such will never be printed here. But just because it doesn’t make sense doesn’t make it wrong. I suppose we can assume that if new features are added, they will have a similar length like the ones we have now. (Well, it does make “60” a weirdly specific number, but whatever.) ((I’d just have created an empty string. Considering this is part of the explanation for a fatal error, nobody cares whether this function takes a couple of microseconds more. Or maybe a length of 47, so it can definitely hold exactly one incompatible feature name.)) > Suggested-by: Philippe Mathieu-Daudé <[email protected]> > Signed-off-by: Alberto Garcia <[email protected]> > Reviewed-by: Alex Bennée <[email protected]> > --- > block/qcow2.c | 23 +++++++++++------------ > 1 file changed, 11 insertions(+), 12 deletions(-) Thanks, applied to my block branch: https://git.xanclic.moe/XanClic/qemu/commits/branch/block > diff --git a/block/qcow2.c b/block/qcow2.c > index cef9d72b3a..e29fc07068 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c [...] > @@ -470,14 +469,14 @@ static void report_unsupported_feature(Error **errp, > Qcow2Feature *table, > } > > if (mask) { > - old = features; > - features = g_strdup_printf("%s%sUnknown incompatible feature: %" > PRIx64, > - old, *old ? ", " : "", mask); > - g_free(old); > + if (features->len > 0) { > + g_string_append(features, ", "); > + } > + g_string_append_printf(features, > + "Unknown incompatible feature: %" PRIx64, > mask); Existing problem: This can lead to stuff like “Unsupported qcow2 feature(s): feat1, feat2, Unknown incompatible feature: 0xec0” (i.e., singular “feature” when there are multiple unknown features, and capitalization of “Unknown”) O:-) But whatever. It’s unlikely there’s going to be more than one incompatible feature, and it’s extremely unlikely it won’t have a string description. Max
signature.asc
Description: OpenPGP digital signature
