2010/12/14 Eric Blake <eblake@xxxxxxxxxx>:
> On 12/06/2010 01:42 PM, Matthias Bolte wrote:
>> ---
>> Âsrc/esx/esx_storage_driver.c  | Â198 +++++++++++++++++++++++++++++++++++++++-
>> Âsrc/esx/esx_vi_generator.input | Â 11 ++
>> Â2 files changed, 208 insertions(+), 1 deletions(-)
>
> ACK. Looks clean; but maybe one suggestion if you want:
>
>> + Â Âchar *key = NULL;
>
>> + Â Â Â Âif (priv->primary->hasQueryVirtualDiskUuid) {
>> + Â Â Â Â Â Âif (VIR_ALLOC_N(key, VIR_UUID_STRING_BUFLEN) < 0) {
>> + Â Â Â Â Â Â Â ÂvirReportOOMError();
>> + Â Â Â Â Â Â Â Âgoto cleanup;
>> + Â Â Â Â Â Â}
>
> VIR_UUID_STRING_BUFLEN is small enough that you can stack-allocate key,
> and avoid one place of malloc() failure:
>
> char key[VIR_UUID_STRING_BUFLEN];
>
> Up to you if you want to make that change, or check in as-is.
>
Yes, if you only look at the if branch then stack allocating the key
makes sense, but when you look at the else branch you'll see that the
key can also be a copy of the datastore path. In that case a stack
allocated key might be a bad idea if the path is longer than
VIR_UUID_STRING_BUFLEN - 1.
Pushing it unchanged. Thanks.
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list