[PATCH 00/12] use g_autoptr for all DIR*

[Laine Stump]

On 10/28/20 7:39 AM, Daniel Henrique Barboza wrote:
> Hey,
> 
> On 10/27/20 10:35 PM, Laine Stump wrote:
>> I don't even remember why I started looking at this. I think that
>> again I was considering changing some function, and making the DIR* in
>> that function autoclose was a prerequisite for a reasonably clean
>> addition to the function. I eventually gave up on the other plan
>> (probably because it was a bad idea), but decided that the DIR*'s
>> really did need to autoclose.
>>
>> In the end, all uses of DIR* could be easily converted to use
>> g_autoptr.
> 
> 
> I think you created this series in parallel with your "node_device: fix 
> leak
> of DIR*" patch, right?.

No. This series has been building up for a couple weeks, but the leak 
just came up 2 days ago and I didn't write the patch for it until I had 
finished and posted this series. But the node-device bugfix needs to be 
pushed before the next release, while this series won't be pushed until 
after the release, so the bugfix had to be based on the current state of 
upstream, rather than stacked on top of other work.

> Because you're not changing 'node_device_udev.c' 
> anywhere in
> this series, meaning that the code base you used still have the DIR leak in
> udevGetVDPACharDev(). The code base didn't have the added 
> VIR_DIR_CLOSE() calls there
> to fix the leak, and I believe you forgot to take that into account 
> here. The end result
> is that the leak fix patch will break after patch 10 removes the 
> VIR_DIR_CLOSE()
> macro.

Yep. It wasn't forgotten, just done in a different order than you 
assumed, and both had to be based on current upstream rather than 
dependent on each other.

> 
> A quick fix is simply using "node_device: fix leak of DIR*" as the first
> patch of this series, then you can handle the removal of VIR_DIR_CLOSE()
> and g_autoptr() for that case in patch 8.

Yep, that's what I'd planned to do.

> There's no cleanup labels to
> be added there, so it's a trivial removal of the two VIR_DIR_CLOSE()
> calls and turning the DIR var to g_autoptr().

> 
> Assuming you go with that (and fix my whitespace nit in patch 12! :P ), 
> feel
> free to add my R-b in all patches:

Okay, thanks!

> 
> 
> Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>