Re: [libvirt] [PATCH] esx: Handle non-UTF-8 encoded VMX files
2010/10/19 Daniel Veillard <veillard(a)redhat.com>:
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