On Wed, Dec 10, 2008 at 07:16:42AM -0500, Itamar Heim wrote: > > -----Original Message----- > > From: Daniel Veillard [mailto:veillard@xxxxxxxxxx] > > Looks overall fine to me, the syntactic construct sounds generic > > enough. > > I would still suggest 2 more changes: > > - in virDomainGraphicsDefParseXML check the values coming from > > the XML, I guess we should accept only "true" and "false" > > and raise an error, otherwise one may pass --fullscreen=foo > > on the command line ! > [IH] the qemu command line can only have -full-screen if the attribute is > true, no other option, but I can lookup the code checking the input you > mentioned. Oh right, I saw the STREQ but it's better to report the problem when parsing IMHO. > > - drop stdbool.h and use int instead of bool, this doesn't really > > gain anything and causes some troubles (e.g. with CIL) > [IH] I was just trying to follow the exiting conventions in the code... > see src/cgroup.c > Which should it be? use int, really... > > > > Also if you could set your mail agent to set a mime-type of text/plain > > on the diff or patch attachemnts, that would allow to comment them > > in-line, which allows for faster feedback loop in the future. > > [IH] will try to remember for patches. thanks :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list