On Thu, 2017-07-20 at 15:46 +0200, Peter Krempa wrote: > > > - virBufferAsprintf(buf, "<numad nodeset='%s'/>\n", nodeset); > > > + if (priv->autoCpuset && > > > + !((cpuset = virBitmapFormat(priv->autoCpuset)))) > > > + goto cleanup; > > > > The previous implementation didn't format the <numad> > > element at all if there was nodeset, whereas the new one > > will always format it. You could add > > > > if (!nodeset && !cpuset) > > goto cleanup; > > If you call virBitmapFormat on an empty or NULL bitmap you still get a > (empty) string allocated so that condition is basically identical to the > one that's already there due to how the bitmaps are formatted: > > if (!priv->autoNodeset && !priv->autoCpuset) > return 0; > > if (priv->autoNodeset && > !((nodeset = virBitmapFormat(priv->autoNodeset)))) > goto cleanup; > > if (priv->autoCpuset && > !((cpuset = virBitmapFormat(priv->autoCpuset)))) > goto cleanup; Oh, you're right. Nevermind then. > > > - if (!nodeset) > > > + if (!nodeset && !cpuset) > > > return 0; > > > > I don't think you want this hunk. > > > > With the new condition, you will perform an early exit only > > if both nodeset and cpuset are NULL; if nodeset is NULL but > > cpuset isn't, the first call to virBitmapParse() a few lines > > below will error out. > > That shouldn't ever happen (tm) :D > > I'll can add a condition that if nodeset is not in the XML the parsing > will be skipped. So in that case only cpuset can be present (for future > use). > > I'll also add a note that accessing autoNodeset in the else branch of if > (cpuset) is safe. Works for me :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
