On Fri, Feb 22, 2019 at 05:04:37PM +0100, Peter Krempa wrote:
VIR_DEFINE_AUTOPTR_FUNC defines a function which is supposed to free
the
resources for the given VIR_AUTOPTR variable. Given that the cleanup
function is executed when the stack frame is being destroyed it does not
make much sense to set the pointer to NULL.
Although correct, the current code is correct too and I really don't see any
benefit behind removing a single line where you reset a variable to NULL, if
anything, this adds more safety regardless of scenario (defensive programming
in general).
Originally, I wanted to argue, that this might introduce an issue in loops if
you pass the temporary variable to a function by reference and then exiting on
failure would free some garbage, but that's irrelevant since Sukrit introduced
a syntax check to ensure, VIR_AUTO variables are always initialized.
Making the inline function contain less code also decreases the
possibility that for a given type the inlining will be declined by the
compiler due to increasing code size.
I honestly doubt that re-setting a variable to NULL will have any impact on
this and it's such a trivial operation that the compiler might optimize this
for you in some way.
I think this patch should be dropped from the series and left as is.
Erik