Re: [PATCH] esx: Handle non-UTF-8 encoded VMX files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2010/10/19 Daniel Veillard <veillard@xxxxxxxxxx>:
> On Mon, Oct 18, 2010 at 10:37:31AM -0600, Eric Blake wrote:
>> On 10/18/2010 10:22 AM, Laine Stump wrote:
>> >>>+
>> >>>+ if (result == NULL) {
>> >>>+ virReportOOMError();
>> >>>+ goto cleanup;
>> >>>+ }
>> >>>+
>> >>>+ cleanup:
>> >>
>> >>Is that last goto necessary, since the label is reached next anyways?
>> >
>> >Maybe this is a defensive goto, assuming that someday someone may add
>> >extra code after that if clause. And for the moment it's just a single
>> >jmp that gets optimized out. (kind of like putting a break on the last
>> >case of a switch statement. It doesn't do anything now, but may save
>> >your bacon later if Ralph Wiggum goes playing in your code ;-))
>>
>> Fair enough :) ÂAlthough one would hope that our review process
>> exists to shield us somewhat from whatever Ralph Wiggum's coding
>> style throws at us :)
>
> ÂWell that's not my style either, but ACK, please push :-)
> Now I should fix libxml2 to support CP1252 event when iconv is not used
> (may happen on Windows), but that's a small separate issue :-)
>
> Daniel
>

I just wanted to be defensive here and use the common style and
emphasis that this is an error condition. But this if condition and
goto gets removed now, because Daniel suggested on IRC to steal the
content of the buffer instead of strdup'ing it:


diff --git a/src/esx/esx_util.c b/src/esx/esx_util.c
index 24b931f..9ab0f70 100644
--- a/src/esx/esx_util.c
+++ b/src/esx/esx_util.c
@@ -847,12 +847,8 @@ esxUtil_ConvertToUTF8(const char *encoding, const
char *string)
         goto cleanup;
     }

-    result = strdup((const char *)xmlBufferContent(utf8));
-
-    if (result == NULL) {
-        virReportOOMError();
-        goto cleanup;
-    }
+    result = (char *)utf8->content;
+    utf8->content = NULL;

   cleanup:
     xmlCharEncCloseFunc(handler);


Thanks, pushed with this diff folded in.

Matthias

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]