macro: introduce TAKE_PTR() macro

This macro will read a pointer of any type, return it, and set the
pointer to NULL. This is useful as an explicit concept of passing
ownership of a memory area between pointers.

This takes inspiration from Rust:

https://doc.rust-lang.org/std/option/enum.Option.html#method.take

and was suggested by Alan Jenkins (@sourcejedi).

It drops ~160 lines of code from our codebase, which makes me like it.
Also, I think it clarifies passing of ownership, and thus helps
readability a bit (at least for the initiated who know the new macro)
This commit is contained in:
Lennart Poettering
2018-03-22 16:53:26 +01:00
parent 1147eef0b6
commit ae2a15bc14
93 changed files with 215 additions and 379 deletions

14
coccinelle/take-ptr.cocci Normal file
View File

@@ -0,0 +1,14 @@
@@
local idexpression p;
expression q;
@@
- p = q;
- q = NULL;
- return p;
+ return TAKE_PTR(q);
@@
expression p, q;
@@
- p = q;
- q = NULL;
+ p = TAKE_PTR(q);

View File

@@ -130,3 +130,12 @@ void* greedy_realloc0(void **p, size_t *allocated, size_t need, size_t size);
_new_ = alloca_align(_size_, (align)); \
(void*)memset(_new_, 0, _size_); \
})
/* Takes inspiration from Rusts's Option::take() method: reads and returns a pointer, but at the same time resets it to
* NULL. See: https://doc.rust-lang.org/std/option/enum.Option.html#method.take */
#define TAKE_PTR(ptr) \
({ \
typeof(ptr) _ptr_ = (ptr); \
(ptr) = NULL; \
_ptr_; \
})

View File

@@ -103,8 +103,7 @@ int capability_set_to_string_alloc(uint64_t set, char **s) {
str[n > 0 ? n - 1 : 0] = '\0'; /* truncate the last space, if it's there */
*s = str;
str = NULL;
*s = TAKE_PTR(str);
return 0;
}

View File

@@ -1424,10 +1424,9 @@ int cg_pid_get_path_shifted(pid_t pid, const char *root, char **cgroup) {
if (r < 0)
return r;
if (c == raw) {
*cgroup = raw;
raw = NULL;
} else {
if (c == raw)
*cgroup = TAKE_PTR(raw);
else {
char *n;
n = strdup(c);
@@ -2010,8 +2009,7 @@ int cg_slice_to_path(const char *unit, char **ret) {
if (!strextend(&s, e, NULL))
return -ENOMEM;
*ret = s;
s = NULL;
*ret = TAKE_PTR(s);
return 0;
}
@@ -2301,8 +2299,7 @@ int cg_mask_to_string(CGroupMask mask, char **ret) {
assert(s);
s[n] = 0;
*ret = s;
s = NULL;
*ret = TAKE_PTR(s);
return 0;
}

View File

@@ -194,8 +194,7 @@ finish:
finish_force_next:
s[sz] = 0;
*ret = s;
s = NULL;
*ret = TAKE_PTR(s);
return 1;
}

View File

@@ -330,8 +330,7 @@ int read_full_stream(FILE *f, char **contents, size_t *size) {
}
buf[l] = 0;
*contents = buf;
buf = NULL; /* do not free */
*contents = TAKE_PTR(buf);
if (size)
*size = l;
@@ -1432,8 +1431,7 @@ int open_tmpfile_linkable(const char *target, int flags, char **ret_path) {
if (fd < 0)
return -errno;
*ret_path = tmp;
tmp = NULL;
*ret_path = TAKE_PTR(tmp);
return fd;
}
@@ -1519,8 +1517,7 @@ int read_nul_string(FILE *f, char **ret) {
return -ENOMEM;
}
*ret = x;
x = NULL;
*ret = TAKE_PTR(x);
return 0;
}

View File

@@ -458,10 +458,8 @@ int get_files_in_directory(const char *path, char ***list) {
n++;
}
if (list) {
*list = l;
l = NULL; /* avoid freeing */
}
if (list)
*list = TAKE_PTR(l);
return n;
}
@@ -838,10 +836,9 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
}
/* If this is not a symlink, then let's just add the name we read to what we already verified. */
if (!done) {
done = first;
first = NULL;
} else {
if (!done)
done = TAKE_PTR(first);
else {
/* If done is "/", as first also contains slash at the head, then remove this redundant slash. */
if (streq(done, "/"))
*done = '\0';
@@ -863,10 +860,8 @@ int chase_symlinks(const char *path, const char *original_root, unsigned flags,
return -ENOMEM;
}
if (ret) {
*ret = done;
done = NULL;
}
if (ret)
*ret = TAKE_PTR(done);
if (flags & CHASE_OPEN) {
int q;

View File

@@ -341,8 +341,7 @@ int get_keymaps(char ***ret) {
strv_sort(l);
*ret = l;
l = NULL;
*ret = TAKE_PTR(l);
return 0;
}

View File

@@ -81,10 +81,8 @@ int name_to_handle_at_loop(
if (name_to_handle_at(fd, path, h, &mnt_id, flags) >= 0) {
if (ret_handle) {
*ret_handle = h;
h = NULL;
}
if (ret_handle)
*ret_handle = TAKE_PTR(h);
if (ret_mnt_id)
*ret_mnt_id = mnt_id;
@@ -951,8 +949,7 @@ int mount_option_mangle(
}
*ret_mount_flags = mount_flags;
*ret_remaining_options = ret;
ret = NULL;
*ret_remaining_options = TAKE_PTR(ret);
return 0;
}

View File

@@ -324,8 +324,7 @@ int parse_syscall_and_errno(const char *in, char **name, int *error) {
return -EINVAL;
*error = e;
*name = n;
n = NULL;
*name = TAKE_PTR(n);
return 0;
}

View File

@@ -290,8 +290,7 @@ char **path_strv_resolve(char **l, const char *root) {
r = chase_symlinks(t, root, 0, &u);
if (r == -ENOENT) {
if (root) {
u = orig;
orig = NULL;
u = TAKE_PTR(orig);
free(t);
} else
u = t;

View File

@@ -204,10 +204,8 @@ int proc_cmdline_get_key(const char *key, unsigned flags, char **value) {
}
}
if (value) {
*value = ret;
ret = NULL;
}
if (value)
*value = TAKE_PTR(ret);
return found;
}

View File

@@ -623,8 +623,7 @@ int get_process_environ(pid_t pid, char **env) {
} else
outcome[sz] = '\0';
*env = outcome;
outcome = NULL;
*env = TAKE_PTR(outcome);
return 0;
}

View File

@@ -48,8 +48,7 @@ int secure_bits_to_string_alloc(int i, char **s) {
if (len != 0)
str[len - 1] = '\0';
*s = str;
str = NULL;
*s = TAKE_PTR(str);
return 0;
}

View File

@@ -985,8 +985,7 @@ int getpeersec(int fd, char **ret) {
if (isempty(s))
return -EOPNOTSUPP;
*ret = s;
s = NULL;
*ret = TAKE_PTR(s);
return 0;
}

View File

@@ -341,8 +341,7 @@ int strv_split_extract(char ***t, const char *s, const char *separators, Extract
if (!GREEDY_REALLOC(l, allocated, n + 2))
return -ENOMEM;
l[n++] = word;
word = NULL;
l[n++] = TAKE_PTR(word);
l[n] = NULL;
}
@@ -353,8 +352,7 @@ int strv_split_extract(char ***t, const char *s, const char *separators, Extract
return -ENOMEM;
}
*t = l;
l = NULL;
*t = TAKE_PTR(l);
return (int) n;
}

View File

@@ -707,10 +707,9 @@ int vtnr_from_tty(const char *tty) {
tty = active;
}
if (tty == active) {
*ret = active;
active = NULL;
} else {
if (tty == active)
*ret = TAKE_PTR(active);
else {
char *tmp;
tmp = strdup(tty);
@@ -778,8 +777,7 @@ int get_kernel_consoles(char ***ret) {
goto fallback;
}
*ret = l;
l = NULL;
*ret = TAKE_PTR(l);
return 0;
@@ -788,8 +786,7 @@ fallback:
if (r < 0)
return r;
*ret = l;
l = NULL;
*ret = TAKE_PTR(l);
return 0;
}

View File

@@ -363,8 +363,7 @@ int unit_name_unescape(const char *f, char **ret) {
*t = 0;
*ret = r;
r = NULL;
*ret = TAKE_PTR(r);
return 0;
}

View File

@@ -275,10 +275,9 @@ static int parse_xml_node(Context *context, const char *prefix, unsigned n_depth
free(node_path);
if (name[0] == '/') {
node_path = name;
name = NULL;
} else {
if (name[0] == '/')
node_path = TAKE_PTR(name);
else {
if (endswith(prefix, "/"))
node_path = strappend(prefix, name);

View File

@@ -1381,8 +1381,7 @@ int unit_set_cgroup_path(Unit *u, const char *path) {
unit_release_cgroup(u);
u->cgroup_path = p;
p = NULL;
u->cgroup_path = TAKE_PTR(p);
return 1;
}

Some files were not shown because too many files have changed in this diff Show More