Re: [PATCH] domain-conf: escape string for socket attribute

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

 



On Mon, Aug 31, 2015 at 05:25:35PM +0200, Martin Kletzander wrote:
> On Mon, Aug 31, 2015 at 03:37:01PM +0200, Pavel Hrdina wrote:
> >Commit d091518b tried to escape all strings in produced XML, but missed
> >this one.
> >
> >Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> >---
> > src/conf/domain_conf.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> >index c5e9653..56f9460 100644
> >--- a/src/conf/domain_conf.c
> >+++ b/src/conf/domain_conf.c
> >@@ -21060,8 +21060,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
> >     case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
> >         if (def->data.vnc.socket) {
> >             if (def->data.vnc.socket)
> >-                virBufferAsprintf(buf, " socket='%s'",
> >-                                  def->data.vnc.socket);
> >+                virBufferEscapeString(buf, " socket='%s'",
> >+                                      def->data.vnc.socket);
> 
> I'd ACK it, but there's something weird going on, even though it's
> pre-existing.  The check for 'def->data.vnc.socket' is duplicated,
> that makes no sense, how did it got there?  Not to mention, that the
> 'EscapeString' handles NULLs gracefully, so no check needs to be there
> for it.  Maybe this could be cleaned up a bit?

Oh, I didn't noticed that at all.  I guess, that I've just blindly changed the
function.  Sure, I'll clean it and send v2.

Thanks

> 
> >         } else {
> >             if (def->data.vnc.port &&
> >                 (!def->data.vnc.autoport || !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)))
> >--
> >2.5.0
> >
> >--
> >libvir-list mailing list
> >libvir-list@xxxxxxxxxx
> >https://www.redhat.com/mailman/listinfo/libvir-list


--
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]