On Tue, Sep 11, 2012 at 03:35:45PM -0600, Eric Blake wrote: > On 09/11/2012 08:11 AM, Daniel P. Berrange wrote: > > From: "Daniel P. Berrange" <berrange@xxxxxxxxxx> > > > > Add an API allowing flags from one virBitmapPtr to be copied > > into another instance. > > > > Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx> > > --- > > src/libvirt_private.syms | 1 + > > src/util/bitmap.c | 19 +++++++++++++++++++ > > src/util/bitmap.h | 6 ++++++ > > 3 files changed, 26 insertions(+) > > This conflicts with a patch proposed by Hu Tao > (https://www.redhat.com/archives/libvir-list/2012-September/msg00317.html). > > > + > > +int virBitmapCopy(virBitmapPtr dst, virBitmapPtr src) > > This requires you to pre-create the copy, so a better name might be > virBitmapCopyInto(), and then rebase Hu's patch, so that his function > signature: > > +/** > + * virBitmapCopy: > + * @src: the source bitmap. > + * > + * Makes a copy of bitmap @src. > + * > + * returns the copied bitmap on success, or NULL otherwise. Caller > + * should call virBitmapFree to free the returned bitmap. > + */ > +virBitmapPtr virBitmapCopy(virBitmapPtr src) > > would then call into your virBitmapCopyInto for fewer lines of > implementation. > > That is, I like Hu's semantics better of creating the copy, but I still > think your function is useful, if given the correct name. Actually I think my function has the right name already. Functions which create new instances should have 'New' in their name, eg virBitmapPtr virBitmapNewCopy(virBitmapPtr src) Unless you object to this suggestion, I'll push my patch and let Hu rebase against it. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list