X Tutup
Skip to content

Commit fd421c4

Browse files
committed
tree-wide: reset the cleaned-up variable in cleanup functions
If the cleanup function returns the appropriate type, use that to reset the variable. For other functions (usually the foreign ones which return void), add an explicit value to reset to. This causes a bit of code churn, but I think it might be worth it. In a following patch static destructors will be called from a fuzzer, and this change allows them to be called multiple times. But I think such a change might help with detecting unitialized code reuse too. We hit various bugs like this, and things are more obvious when a pointer has been set to NULL. I was worried whether this change increases text size, but it doesn't seem to: -Dbuildtype=debug: before "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494520 Feb 16 15:06 build/systemd* after "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494576 Feb 16 15:10 build/systemd* now: -rwxrwxr-x 1 zbyszek zbyszek 4117672 Feb 16 14:36 build/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 4494640 Feb 16 15:15 build/systemd* -Dbuildtype=release: before "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:09 build-rawhide/systemd* after "tree-wide: return NULL from freeing functions": -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:10 build-rawhide/systemd* now: -rwxrwxr-x 1 zbyszek zbyszek 5252256 Feb 14 14:47 build-rawhide/libsystemd.so.0.30.0* -rwxrwxr-x 1 zbyszek zbyszek 1834184 Feb 16 15:16 build-rawhide/systemd* I would expect that the compiler would be able to elide the setting of a variable if the variable is never used again. And this seems to be the case: in optimized builds there is no change in size whatsoever. And the change in size in unoptimized build is negligible. Something strange is happening with size of libsystemd: it's bigger in optimized builds. Something to figure out, but unrelated to this patch.
1 parent 75db809 commit fd421c4

31 files changed

+66
-56
lines changed

src/basic/capability-util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ int drop_privileges(uid_t uid, gid_t gid, uint64_t keep_capabilities);
2525

2626
int drop_capability(cap_value_t cv);
2727

28-
DEFINE_TRIVIAL_CLEANUP_FUNC(cap_t, cap_free);
28+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(cap_t, cap_free, NULL);
2929
#define _cleanup_cap_free_ _cleanup_(cap_freep)
3030

3131
static inline void cap_free_charpp(char **p) {

src/basic/dlfcn-util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
#include "macro.h"
77

8-
DEFINE_TRIVIAL_CLEANUP_FUNC(void*, dlclose);
8+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(void*, dlclose, NULL);
99

1010
int dlsym_many_and_warn(void *dl, int level, ...);
1111

src/basic/fd-util.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ static inline void fclosep(FILE **f) {
4141
safe_fclose(*f);
4242
}
4343

44-
DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, pclose);
45-
DEFINE_TRIVIAL_CLEANUP_FUNC(DIR*, closedir);
44+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(FILE*, pclose, NULL);
45+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(DIR*, closedir, NULL);
4646

4747
#define _cleanup_close_ _cleanup_(closep)
4848
#define _cleanup_fclose_ _cleanup_(fclosep)

src/basic/fileio.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1146,7 +1146,7 @@ static EndOfLineMarker categorize_eol(char c, ReadLineFlags flags) {
11461146
return EOL_NONE;
11471147
}
11481148

1149-
DEFINE_TRIVIAL_CLEANUP_FUNC(FILE*, funlockfile);
1149+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(FILE*, funlockfile, NULL);
11501150

11511151
int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
11521152
size_t n = 0, allocated = 0, count = 0;

src/basic/gcrypt-util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
void initialize_libgcrypt(bool secmem);
1515
int string_hashsum(const char *s, size_t len, int md_algorithm, char **out);
1616

17-
DEFINE_TRIVIAL_CLEANUP_FUNC(gcry_md_hd_t, gcry_md_close);
17+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(gcry_md_hd_t, gcry_md_close, NULL);
1818
#endif
1919

2020
static inline int string_hashsum_sha224(const char *s, size_t len, char **out) {

src/basic/macro.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,10 +405,20 @@ static inline int __coverity_check_and_return__(int condition) {
405405
func(p); \
406406
}
407407

408+
/* When func() returns the void value (NULL, -1, …) of the appropriate type */
408409
#define DEFINE_TRIVIAL_CLEANUP_FUNC(type, func) \
409410
static inline void func##p(type *p) { \
410411
if (*p) \
412+
*p = func(*p); \
413+
}
414+
415+
/* When func() doesn't return the appropriate type, set variable to empty afterwards */
416+
#define DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(type, func, empty) \
417+
static inline void func##p(type *p) { \
418+
if (*p != (empty)) { \
411419
func(*p); \
420+
*p = (empty); \
421+
} \
412422
}
413423

414424
#define _DEFINE_TRIVIAL_REF_FUNC(type, name, scope) \

src/basic/selinux-util.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
#include "time-util.h"
3030

3131
#if HAVE_SELINUX
32-
DEFINE_TRIVIAL_CLEANUP_FUNC(context_t, context_free);
32+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(context_t, context_free, NULL);
3333
#define _cleanup_context_free_ _cleanup_(context_freep)
3434

3535
static int mac_selinux_reload(int seqno);

src/basic/selinux-util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
#if HAVE_SELINUX
1212
#include <selinux/selinux.h>
1313

14-
DEFINE_TRIVIAL_CLEANUP_FUNC(char*, freecon);
14+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(char*, freecon, NULL);
1515
#define _cleanup_freecon_ _cleanup_(freeconp)
1616
#endif
1717

src/core/apparmor-setup.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,17 @@
1616
#include "strv.h"
1717

1818
#if HAVE_APPARMOR
19-
DEFINE_TRIVIAL_CLEANUP_FUNC(aa_policy_cache *, aa_policy_cache_unref);
20-
DEFINE_TRIVIAL_CLEANUP_FUNC(aa_features *, aa_features_unref);
19+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(aa_policy_cache *, aa_policy_cache_unref, NULL);
20+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(aa_features *, aa_features_unref, NULL);
2121
#endif
2222

2323
int mac_apparmor_setup(void) {
2424
#if HAVE_APPARMOR
25-
int r;
2625
_cleanup_(aa_policy_cache_unrefp) aa_policy_cache *policy_cache = NULL;
2726
_cleanup_(aa_features_unrefp) aa_features *features = NULL;
2827
const char *current_file;
2928
_cleanup_free_ char *current_profile = NULL, *cache_dir_path = NULL;
29+
int r;
3030

3131
if (!mac_apparmor_use()) {
3232
log_debug("AppArmor either not supported by the kernel or disabled.");

src/core/socket.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1611,7 +1611,7 @@ static int socket_address_listen_in_cgroup(
16111611
return fd;
16121612
}
16131613

1614-
DEFINE_TRIVIAL_CLEANUP_FUNC(Socket *, socket_close_fds);
1614+
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(Socket *, socket_close_fds, NULL);
16151615

16161616
static int socket_open_fds(Socket *orig_s) {
16171617
_cleanup_(socket_close_fdsp) Socket *s = orig_s;

0 commit comments

Comments
 (0)
X Tutup