On Tue, Sep 04, 2012 at 09:03:22PM +0000, Kyle Mestery (kmestery) wrote: > On Aug 31, 2012, at 9:09 AM, Daniel Veillard wrote: > > On Fri, Aug 31, 2012 at 01:32:34PM +0000, Kyle Mestery (kmestery) wrote: > >> On Aug 30, 2012, at 10:23 PM, Daniel Veillard wrote: > >>> On Thu, Aug 30, 2012 at 04:38:06PM -0400, Kyle Mestery wrote: > > [...] > >>> Still there is something which looks wrong, if we don't have a profileID > >>> why do we end up with "" instead of NULL ? I'm seeing various tests for > >>> profileID[0] over conf/*.c and util/*.c , and that sounds wrong to me. > >>> if there is no data, store NULL ! Then test for profileID instead of > >>> profileID[0]. Then there is no risk of a crash because abscence of data > >>> led to NULL instead of an empty string, the code is more resilient ! > >>> > >>> I expect a followup patch cleaning this up, but after 0.10.1 ... > >>> thanks ! > >>> > >> Thanks Daniel, I'll work on the followup patch today. > > > > No hurry, because I just rolled 0.10.1 out so that won't make it > > (and it's not urgent). Giving 0.10.1 a try would be nice too, > > > > thanks ! > > > > Daniel > > > Hi Daniel: > > Picking this back up. struct _virNetDevVPortProfile contains the following: > > /* this member is used when virtPortType == 802.1Qbh|openvswitch */ > /* this is a null-terminated character string */ > char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; > > To address your comments around checking for profileID[0], we could make > profileID here a pointer, and allocate it when we allocate a struct _virNetDevVPortProfile > object. But to me, having a fixed length string in this structure doesn't seem wrong. > Copying Laine here as well for his comments, but I'm inclined to leave things as they > are. Hum ... okay, 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