[systemd-devel] [PATCH 0/4] looking at _cleanup_free_ usage

[Andreas Henriksson]

Hello all!

I was recently bitten by a problem which I found the solution to in
http://lists.freedesktop.org/archives/systemd-commits/2013-October/004421.html

The second patch there made me curious to look at more occurances.

I do not have prior experience with _cleanup_free_ but as I understand
it in general the pointer gets free()d when it goes out of scope
and thus must be initialized before that happens to avoid memory corruption.

This thread contains patches for the occurences I found which seems
to violate that.
Please review them carefully!!!
(I have not tried to trigger the actual codepaths.)



Feel free to also follow up with general discussion on best practices.

Should CODING_STYLE also contain a blurb next to the recommendation
to use _cleanup_free_ that reminds/warns people it must be initialized
before it goes out of scope?

Should helper function patterns be changed to set *ret = NULL early
on to try to prevent cases where it does not initialize a variable
on error cases?
ie.

int myhelper(..., char **ret) {
	int x, y, z;

	assert(...);

	// This is just for extra safety so the value always gets initialized.
	*ret = NULL;

	...

	if (x != y)
		return -EEXAMPLE;

	...

	*ret = strdup("example");
	return 0;
}


Is the same problem also affecting other _cleanup_whatever_ variants?

Is there a general solution in the macro itself to make sure
the variable it is used on always gets initialized?

Should CODING_STYLE be changed to recommend this:
	_cleanup_free_ char *myptr = strdup("example");
.. instead of this:
	_cleanup_free_ char *myptr;

	myptr = strdup("example");
... to make it easier to spot (or grep for) errors?

Are there any static code analyzers that can catch these errors that can
be used to avoid relying on manual reviews catching these errors?

Any other ideas?


Have a great weekend!

Regards,
Andreas Henriksson


Andreas Henriksson (4):
  install: fix invalid free() in unit_file_mask()
  core: fix invalid free() in killall()
  sd-dhcp-client: fix invalid free() in client_send_request()
  udev: fix invalid free() in enable_name_policy()

 src/core/killall.c                      | 2 +-
 src/libsystemd-network/sd-dhcp-client.c | 2 +-
 src/shared/install.c                    | 2 +-
 src/udev/net/link-config.c              | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.0.0