Skip to content

Commit

Permalink
Merge branch 'ab/free-and-null'
Browse files Browse the repository at this point in the history
A common pattern to free a piece of memory and assign NULL to the
pointer that used to point at it has been replaced with a new
FREE_AND_NULL() macro.

* ab/free-and-null:
  *.[ch] refactoring: make use of the FREE_AND_NULL() macro
  coccinelle: make use of the "expression" FREE_AND_NULL() rule
  coccinelle: add a rule to make "expression" code use FREE_AND_NULL()
  coccinelle: make use of the "type" FREE_AND_NULL() rule
  coccinelle: add a rule to make "type" code use FREE_AND_NULL()
  git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL
  • Loading branch information
gitster committed Jun 24, 2017
2 parents cda4ba3 + 88ce3ef commit 50f03c6
Show file tree
Hide file tree
Showing 49 changed files with 117 additions and 195 deletions.
6 changes: 2 additions & 4 deletions alias.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ int split_cmdline(char *cmdline, const char ***argv)
src++;
c = cmdline[src];
if (!c) {
free(*argv);
*argv = NULL;
FREE_AND_NULL(*argv);
return -SPLIT_CMDLINE_BAD_ENDING;
}
}
Expand All @@ -75,8 +74,7 @@ int split_cmdline(char *cmdline, const char ***argv)
cmdline[dst] = 0;

if (quoted) {
free(*argv);
*argv = NULL;
FREE_AND_NULL(*argv);
return -SPLIT_CMDLINE_UNCLOSED_QUOTE;
}

Expand Down
3 changes: 1 addition & 2 deletions apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -3695,8 +3695,7 @@ static int check_preimage(struct apply_state *state,
is_new:
patch->is_new = 1;
patch->is_delete = 0;
free(patch->old_name);
patch->old_name = NULL;
FREE_AND_NULL(patch->old_name);
return 0;
}

Expand Down
6 changes: 2 additions & 4 deletions attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -639,13 +639,11 @@ void attr_check_reset(struct attr_check *check)

void attr_check_clear(struct attr_check *check)
{
free(check->items);
check->items = NULL;
FREE_AND_NULL(check->items);
check->alloc = 0;
check->nr = 0;

free(check->all_attrs);
check->all_attrs = NULL;
FREE_AND_NULL(check->all_attrs);
check->all_attrs_nr = 0;

drop_attr_stack(&check->stack);
Expand Down
3 changes: 1 addition & 2 deletions blame.c
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,7 @@ static void fill_origin_blob(struct diff_options *opt,
static void drop_origin_blob(struct blame_origin *o)
{
if (o->file.ptr) {
free(o->file.ptr);
o->file.ptr = NULL;
FREE_AND_NULL(o->file.ptr);
}
}

Expand Down
3 changes: 1 addition & 2 deletions branch.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ static int find_tracked_branch(struct remote *remote, void *priv)
} else {
free(tracking->spec.src);
if (tracking->src) {
free(tracking->src);
tracking->src = NULL;
FREE_AND_NULL(tracking->src);
}
}
tracking->spec.src = NULL;
Expand Down
18 changes: 5 additions & 13 deletions builtin/am.c
Original file line number Diff line number Diff line change
Expand Up @@ -484,8 +484,7 @@ static int run_applypatch_msg_hook(struct am_state *state)
ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL);

if (!ret) {
free(state->msg);
state->msg = NULL;
FREE_AND_NULL(state->msg);
if (read_commit_msg(state) < 0)
die(_("'%s' was deleted by the applypatch-msg hook"),
am_path(state, "final-commit"));
Expand Down Expand Up @@ -1074,17 +1073,10 @@ static void am_next(struct am_state *state)
{
struct object_id head;

free(state->author_name);
state->author_name = NULL;

free(state->author_email);
state->author_email = NULL;

free(state->author_date);
state->author_date = NULL;

free(state->msg);
state->msg = NULL;
FREE_AND_NULL(state->author_name);
FREE_AND_NULL(state->author_email);
FREE_AND_NULL(state->author_date);
FREE_AND_NULL(state->msg);
state->msg_len = 0;

unlink(am_path(state, "author-script"));
Expand Down
6 changes: 2 additions & 4 deletions builtin/clean.c
Original file line number Diff line number Diff line change
Expand Up @@ -838,8 +838,7 @@ static void interactive_main_loop(void)
int ret;
ret = menus[*chosen].fn();
if (ret != MENU_RETURN_NO_LOOP) {
free(chosen);
chosen = NULL;
FREE_AND_NULL(chosen);
if (!del_list.nr) {
clean_print_color(CLEAN_COLOR_ERROR);
printf_ln(_("No more files to clean, exiting."));
Expand All @@ -852,8 +851,7 @@ static void interactive_main_loop(void)
quit_cmd();
}

free(chosen);
chosen = NULL;
FREE_AND_NULL(chosen);
break;
}
}
Expand Down
6 changes: 2 additions & 4 deletions builtin/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,8 +215,7 @@ static int get_value(const char *key_, const char *regex_)
key_regexp = (regex_t*)xmalloc(sizeof(regex_t));
if (regcomp(key_regexp, key, REG_EXTENDED)) {
error("invalid key pattern: %s", key_);
free(key_regexp);
key_regexp = NULL;
FREE_AND_NULL(key_regexp);
ret = CONFIG_INVALID_PATTERN;
goto free_strings;
}
Expand All @@ -236,8 +235,7 @@ static int get_value(const char *key_, const char *regex_)
regexp = (regex_t*)xmalloc(sizeof(regex_t));
if (regcomp(regexp, regex_, REG_EXTENDED)) {
error("invalid pattern: %s", regex_);
free(regexp);
regexp = NULL;
FREE_AND_NULL(regexp);
ret = CONFIG_INVALID_PATTERN;
goto free_strings;
}
Expand Down
6 changes: 2 additions & 4 deletions builtin/index-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -389,8 +389,7 @@ static struct base_data *alloc_base_data(void)
static void free_base_data(struct base_data *c)
{
if (c->data) {
free(c->data);
c->data = NULL;
FREE_AND_NULL(c->data);
get_thread_data()->base_cache_used -= c->size;
}
}
Expand Down Expand Up @@ -606,8 +605,7 @@ static void *unpack_data(struct object_entry *obj,
git_inflate_end(&stream);
free(inbuf);
if (consume) {
free(data);
data = NULL;
FREE_AND_NULL(data);
}
return data;
}
Expand Down
12 changes: 4 additions & 8 deletions builtin/pack-objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,7 @@ static unsigned long write_no_reuse_object(struct sha1file *f, struct object_ent
* make sure no cached delta data remains from a
* previous attempt before a pack split occurred.
*/
free(entry->delta_data);
entry->delta_data = NULL;
FREE_AND_NULL(entry->delta_data);
entry->z_delta_size = 0;
} else if (entry->delta_data) {
size = entry->delta_size;
Expand Down Expand Up @@ -1376,12 +1375,10 @@ static void cleanup_preferred_base(void)
if (!pbase_tree_cache[i])
continue;
free(pbase_tree_cache[i]->tree_data);
free(pbase_tree_cache[i]);
pbase_tree_cache[i] = NULL;
FREE_AND_NULL(pbase_tree_cache[i]);
}

free(done_pbase_paths);
done_pbase_paths = NULL;
FREE_AND_NULL(done_pbase_paths);
done_pbase_paths_num = done_pbase_paths_alloc = 0;
}

Expand Down Expand Up @@ -1971,8 +1968,7 @@ static unsigned long free_unpacked(struct unpacked *n)
n->index = NULL;
if (n->data) {
freed_mem += n->entry->size;
free(n->data);
n->data = NULL;
FREE_AND_NULL(n->data);
}
n->entry = NULL;
n->depth = 0;
Expand Down
3 changes: 1 addition & 2 deletions builtin/unpack-objects.c
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,7 @@ static void *get_data(unsigned long size)
break;
if (ret != Z_OK) {
error("inflate returned %d", ret);
free(buf);
buf = NULL;
FREE_AND_NULL(buf);
if (!recover)
exit(1);
has_errors = 1;
Expand Down
6 changes: 2 additions & 4 deletions builtin/worktree.c
Original file line number Diff line number Diff line change
Expand Up @@ -300,10 +300,8 @@ static int add_worktree(const char *path, const char *refname,
}

is_junk = 0;
free(junk_work_tree);
free(junk_git_dir);
junk_work_tree = NULL;
junk_git_dir = NULL;
FREE_AND_NULL(junk_work_tree);
FREE_AND_NULL(junk_git_dir);

done:
if (ret || !opts->keep_locked) {
Expand Down
3 changes: 1 addition & 2 deletions commit-slab.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,7 @@ static MAYBE_UNUSED void clear_ ##slabname(struct slabname *s) \
for (i = 0; i < s->slab_count; i++) \
free(s->slab[i]); \
s->slab_count = 0; \
free(s->slab); \
s->slab = NULL; \
FREE_AND_NULL(s->slab); \
} \
\
static MAYBE_UNUSED elemtype *slabname## _at_peek(struct slabname *s, \
Expand Down
3 changes: 1 addition & 2 deletions commit.c
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,7 @@ void free_commit_buffer(struct commit *commit)
{
struct commit_buffer *v = buffer_slab_peek(&buffer_slab, commit);
if (v) {
free(v->buffer);
v->buffer = NULL;
FREE_AND_NULL(v->buffer);
v->size = 0;
}
}
Expand Down
3 changes: 1 addition & 2 deletions config.c
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,7 @@ static int git_config_parse_key_1(const char *key, char **store_key, int *basele

out_free_ret_1:
if (store_key) {
free(*store_key);
*store_key = NULL;
FREE_AND_NULL(*store_key);
}
return -CONFIG_INVALID_KEY;
}
Expand Down
15 changes: 15 additions & 0 deletions contrib/coccinelle/free.cocci
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,18 @@ expression E;
@@
- if (!E)
free(E);

@@
type T;
T *ptr;
@@
- free(ptr);
- ptr = NULL;
+ FREE_AND_NULL(ptr);

@@
expression E;
@@
- free(E);
- E = NULL;
+ FREE_AND_NULL(E);
9 changes: 3 additions & 6 deletions credential.c
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,7 @@ static void credential_apply_config(struct credential *c)
c->configured = 1;

if (!c->use_http_path && proto_is_http(c->protocol)) {
free(c->path);
c->path = NULL;
FREE_AND_NULL(c->path);
}
}

Expand Down Expand Up @@ -315,10 +314,8 @@ void credential_reject(struct credential *c)
for (i = 0; i < c->helpers.nr; i++)
credential_do(c, c->helpers.items[i].string, "erase");

free(c->username);
c->username = NULL;
free(c->password);
c->password = NULL;
FREE_AND_NULL(c->username);
FREE_AND_NULL(c->password);
c->approved = 0;
}

Expand Down
3 changes: 1 addition & 2 deletions diff-lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
free(dpath);
continue;
}
free(dpath);
dpath = NULL;
FREE_AND_NULL(dpath);

/*
* Show the diff for the 'ce' if we found the one
Expand Down
6 changes: 2 additions & 4 deletions diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -1219,8 +1219,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
regfree(ecbdata->diff_words->word_regex);
free(ecbdata->diff_words->word_regex);
}
free(ecbdata->diff_words);
ecbdata->diff_words = NULL;
FREE_AND_NULL(ecbdata->diff_words);
}
}

Expand Down Expand Up @@ -2952,8 +2951,7 @@ void diff_free_filespec_blob(struct diff_filespec *s)
void diff_free_filespec_data(struct diff_filespec *s)
{
diff_free_filespec_blob(s);
free(s->cnt_data);
s->cnt_data = NULL;
FREE_AND_NULL(s->cnt_data);
}

static void prep_temp_blob(const char *path, struct diff_tempfile *temp,
Expand Down
6 changes: 2 additions & 4 deletions diffcore-rename.c
Original file line number Diff line number Diff line change
Expand Up @@ -667,11 +667,9 @@ void diffcore_rename(struct diff_options *options)
for (i = 0; i < rename_dst_nr; i++)
free_filespec(rename_dst[i].two);

free(rename_dst);
rename_dst = NULL;
FREE_AND_NULL(rename_dst);
rename_dst_nr = rename_dst_alloc = 0;
free(rename_src);
rename_src = NULL;
FREE_AND_NULL(rename_src);
rename_src_nr = rename_src_alloc = 0;
return;
}
9 changes: 3 additions & 6 deletions dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -2127,8 +2127,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,
for (i = j = 0; j < dir->nr; j++) {
if (i &&
check_dir_entry_contains(dir->entries[i - 1], dir->entries[j])) {
free(dir->entries[j]);
dir->entries[j] = NULL;
FREE_AND_NULL(dir->entries[j]);
} else {
dir->entries[i++] = dir->entries[j];
}
Expand All @@ -2154,8 +2153,7 @@ int read_directory(struct dir_struct *dir, struct index_state *istate,
dir->untracked->dir_invalidated))
istate->cache_changed |= UNTRACKED_CHANGED;
if (dir->untracked != istate->untracked) {
free(dir->untracked);
dir->untracked = NULL;
FREE_AND_NULL(dir->untracked);
}
}
return dir->nr;
Expand Down Expand Up @@ -2498,8 +2496,7 @@ void write_untracked_extension(struct strbuf *out, struct untracked_cache *untra
strbuf_addbuf(out, &untracked->ident);

strbuf_add(out, ouc, ouc_size(len));
free(ouc);
ouc = NULL;
FREE_AND_NULL(ouc);

if (!untracked->root) {
varint_len = encode_varint(0, varbuf);
Expand Down
6 changes: 2 additions & 4 deletions fast-import.c
Original file line number Diff line number Diff line change
Expand Up @@ -1064,8 +1064,7 @@ static void end_packfile(void)
close(pack_data->pack_fd);
unlink_or_warn(pack_data->pack_name);
}
free(pack_data);
pack_data = NULL;
FREE_AND_NULL(pack_data);
running = 0;

/* We can't carry a delta across packfiles. */
Expand Down Expand Up @@ -1150,8 +1149,7 @@ static int store_object(

/* We cannot carry a delta into the new pack. */
if (delta) {
free(delta);
delta = NULL;
FREE_AND_NULL(delta);

git_deflate_init(&s, pack_compression_level);
s.next_in = (void *)dat->buf;
Expand Down
6 changes: 6 additions & 0 deletions git-compat-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -808,6 +808,12 @@ extern char *xgetcwd(void);
extern FILE *fopen_for_writing(const char *path);
extern FILE *fopen_or_warn(const char *path, const char *mode);

/*
* FREE_AND_NULL(ptr) is like free(ptr) followed by ptr = NULL. Note
* that ptr is used twice, so don't pass e.g. ptr++.
*/
#define FREE_AND_NULL(p) do { free(p); (p) = NULL; } while (0)

#define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))
#define REALLOC_ARRAY(x, alloc) (x) = xrealloc((x), st_mult(sizeof(*(x)), (alloc)))

Expand Down
Loading

0 comments on commit 50f03c6

Please sign in to comment.