Re: [PATCH v2] pci: Use virPCIDeviceAddress in virPCIDevice

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

 



On Tue, 2015-12-15 at 12:27 -0500, Laine Stump wrote:
> On 12/15/2015 05:48 AM, Andrea Bolognani wrote:
> > On Tue, 2015-12-15 at 10:05 +0100, Andrea Bolognani wrote:
> > > @@ -1661,25 +1661,14 @@ virPCIDeviceFree(virPCIDevicePtr dev)
> > >     * @dev: device to get address from
> > >     *
> > >     * Take a PCI device on input and return its PCI address. The
> > > - * caller must free the returned value when no longer needed.
> > > + * returned object is owned by the device and must not be freed.
> > >     *
> > >     * Returns NULL on failure, the device address on success.
> > >     */
> > Not sending a v3 just for this, but if ACKed I'll also squash in
> > 
> > - * Returns NULL on failure, the device address on success.
> > + * Returns: a pointer to the address, which can never be NULL.
> > 
> > to keep the documentation consistent with the actual behaviour.
> 
> Also, the places where this is called no longer need to check for NULL
> and goto cleanup. And beyond that, 2 of the three callers only use the
> returned value a single time, so the code could be shortened even
> further by just using "virPCIDeviceGetAddress(blah)" directly, instead
> of placing that return value into a local, and then calling using the
> local a single time later.
> 
> ACK for the whole patch as long as you remove the check for NULL from
> all the callers of virPCIDeviceGetAddress() (and optionally eliminate
> the temporary variable in the two cases where it is used only once).

v3 here:

  https://www.redhat.com/archives/libvir-list/2015-December/msg00601.html

Hopefully I haven't made any silly mistakes this time :)

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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