On Sun, Oct 01, 2017 at 04:56:03PM +0200, Martin Ågren wrote:
> There is no longer any need to allocate and leak a `struct lock_file`.
> The previous patch addressed an instance where we needed a minor tweak
> alongside the trivial changes.
>
> Deal with the remaining instances where we allocate and leak a struct
> within a single function. Change them to have the `struct lock_file` on
> the stack instead.
>
> These instances were identified by running `git grep "^\s*struct
> lock_file\s*\*"`.
Thanks. These all look pretty straightforward, and as a general rule
this _should_ be a safe conversion with respect to dangling references,
since:
- the tempfile pointers copied onto the global cleanup list are always
allocated, so there's no danger of our stack variables going out of
scope and leaving cruft in the global list.
- when the lock is committed or rolled back, we NULL the pointer after
freeing the tempfile. Nobody should be looking at the lock after
that, but if there is such a bug, we should reliably hit an assert
(if they use the accessors which check for validity) or a segfault
(if somebody tries to dereference it directly).
So it's possible that this subtly introduces a new segfault or
assertion failure, but in that case it would be finding an
_existing_ bug which was previously just reading cruft from the
inactive tempfile struct.
-Peff
