Re: [PATCH v3 06/10] qemu_driver: use VIR_AUTOFREE() with strings 1/3

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

 





On 10/16/19 11:28 AM, Ján Tomko wrote:
On Wed, Oct 16, 2019 at 11:14:05AM -0300, Daniel Henrique Barboza wrote:


On 10/16/19 10:49 AM, Ján Tomko wrote:
On Tue, Oct 15, 2019 at 05:08:48PM -0300, Daniel Henrique Barboza wrote:
Using VIR_AUTOFREE() in all strings of qemu_driver.c make the code
a bit tidier and smaller, sparing VIR_FREE() calls and sometimes a
whole 'cleanup' label.

These patches would look much better split by:
* individual functions (in case you do rework multiple things at once)

Do you mean sending an individual patch for any function that might
have, say, 2+ changes in it? For example, if the same function
was changed to use g_autoptr and g_autofree and perhaps
that causes a label to be dropped, this can be an individual patch?

Yes, but if you convert a lot of functions, that would result in a lot
of patches.

Just checked. There's just a couple of functions I messed around in a
non-trivial way (moved string declarations inside loops to avoid a
VIR_FREE call, lots of VIR_FREE calls being removed and so on).

I can send them in separated patches (perhaps both in the same patch,
I think the functions are related), then proceed with the more trivial
changes of inserting g_autoptr, g_autofree and removing labels in
separated patches each.


DHB



Sending one patch per function is more viable for the cases where you
need to refactor it in order to add some functionality later (see
Jirka's series for example). For mass conversion for the sake of
conversion, one patch per change is better.

Jano



* individual changes, i.e.
 * g_autofree for scalars
 * g_autoptr for pointers and unref
 * possible removal of cleanup labels

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

  Powered by Linux