From 568edcb95a8b86ffd0d267b124896df8ea81307c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 28 Jan 2017 22:38:21 +0100 Subject: add SWAP macro Add a macro for exchanging the values of variables. It allows users to avoid repetition and takes care of the temporary variable for them. It also makes sure that the storage sizes of its two parameters are the same. Its memcpy(1) calls are optimized away by current compilers. Also add a conservative semantic patch for transforming only swaps of variables of the same type. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- contrib/coccinelle/swap.cocci | 28 ++++++++++++++++++++++++++++ git-compat-util.h | 10 ++++++++++ 2 files changed, 38 insertions(+) create mode 100644 contrib/coccinelle/swap.cocci diff --git a/contrib/coccinelle/swap.cocci b/contrib/coccinelle/swap.cocci new file mode 100644 index 0000000000..a0934d1fda --- /dev/null +++ b/contrib/coccinelle/swap.cocci @@ -0,0 +1,28 @@ +@ swap_with_declaration @ +type T; +identifier tmp; +T a, b; +@@ +- T tmp = a; ++ T tmp; ++ tmp = a; + a = b; + b = tmp; + +@ swap @ +type T; +T tmp, a, b; +@@ +- tmp = a; +- a = b; +- b = tmp; ++ SWAP(a, b); + +@ extends swap @ +identifier unused; +@@ + { + ... +- T unused; + ... when != unused + } diff --git a/git-compat-util.h b/git-compat-util.h index 87237b092b..66cd466eea 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -527,6 +527,16 @@ static inline int ends_with(const char *str, const char *suffix) return strip_suffix(str, suffix, &len); } +#define SWAP(a, b) do { \ + void *_swap_a_ptr = &(a); \ + void *_swap_b_ptr = &(b); \ + unsigned char _swap_buffer[sizeof(a)]; \ + memcpy(_swap_buffer, _swap_a_ptr, sizeof(a)); \ + memcpy(_swap_a_ptr, _swap_b_ptr, sizeof(a) + \ + BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b))); \ + memcpy(_swap_b_ptr, _swap_buffer, sizeof(a)); \ +} while (0) + #if defined(NO_MMAP) || defined(USE_WIN32_MMAP) #ifndef PROT_READ -- cgit v1.2.3 From db101991414f4e14b3763b4843dddcca6b31b40b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 28 Jan 2017 22:40:06 +0100 Subject: apply: use SWAP macro Use the exported macro SWAP instead of the file-scoped macro swap and remove the latter's definition. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- apply.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/apply.c b/apply.c index 2ed808d429..0e2caeab9c 100644 --- a/apply.c +++ b/apply.c @@ -2187,29 +2187,20 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si return offset + hdrsize + patchsize; } -#define swap(a,b) myswap((a),(b),sizeof(a)) - -#define myswap(a, b, size) do { \ - unsigned char mytmp[size]; \ - memcpy(mytmp, &a, size); \ - memcpy(&a, &b, size); \ - memcpy(&b, mytmp, size); \ -} while (0) - static void reverse_patches(struct patch *p) { for (; p; p = p->next) { struct fragment *frag = p->fragments; - swap(p->new_name, p->old_name); - swap(p->new_mode, p->old_mode); - swap(p->is_new, p->is_delete); - swap(p->lines_added, p->lines_deleted); - swap(p->old_sha1_prefix, p->new_sha1_prefix); + SWAP(p->new_name, p->old_name); + SWAP(p->new_mode, p->old_mode); + SWAP(p->is_new, p->is_delete); + SWAP(p->lines_added, p->lines_deleted); + SWAP(p->old_sha1_prefix, p->new_sha1_prefix); for (; frag; frag = frag->next) { - swap(frag->newpos, frag->oldpos); - swap(frag->newlines, frag->oldlines); + SWAP(frag->newpos, frag->oldpos); + SWAP(frag->newlines, frag->oldlines); } } } -- cgit v1.2.3 From 35d803bc9a0d21c36b1381f6e42455beeb73b715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 28 Jan 2017 22:40:58 +0100 Subject: use SWAP macro Apply the semantic patch swap.cocci to convert hand-rolled swaps to use the macro SWAP. The resulting code is shorter and easier to read, the object code is effectively unchanged. The patch for object.c had to be hand-edited in order to preserve the comment before the change; Coccinelle tried to eat it for some reason. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- builtin/diff-tree.c | 4 +--- builtin/diff.c | 9 +++------ diff-no-index.c | 3 +-- diff.c | 8 +++----- graph.c | 5 +---- line-range.c | 3 +-- merge-recursive.c | 5 +---- object.c | 4 +--- pack-revindex.c | 5 +---- prio-queue.c | 4 +--- strbuf.h | 4 +--- 11 files changed, 15 insertions(+), 39 deletions(-) diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c index 806dd7a885..8ce00480cd 100644 --- a/builtin/diff-tree.c +++ b/builtin/diff-tree.c @@ -147,9 +147,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix) tree1 = opt->pending.objects[0].item; tree2 = opt->pending.objects[1].item; if (tree2->flags & UNINTERESTING) { - struct object *tmp = tree2; - tree2 = tree1; - tree1 = tmp; + SWAP(tree2, tree1); } diff_tree_sha1(tree1->oid.hash, tree2->oid.hash, diff --git a/builtin/diff.c b/builtin/diff.c index 7f91f6d226..3d64b85337 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -45,12 +45,9 @@ static void stuff_change(struct diff_options *opt, return; if (DIFF_OPT_TST(opt, REVERSE_DIFF)) { - unsigned tmp; - const unsigned char *tmp_u; - const char *tmp_c; - tmp = old_mode; old_mode = new_mode; new_mode = tmp; - tmp_u = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_u; - tmp_c = old_name; old_name = new_name; new_name = tmp_c; + SWAP(old_mode, new_mode); + SWAP(old_sha1, new_sha1); + SWAP(old_name, new_name); } if (opt->prefix && diff --git a/diff-no-index.c b/diff-no-index.c index f420786039..1ae09894d7 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -186,9 +186,8 @@ static int queue_diff(struct diff_options *o, if (DIFF_OPT_TST(o, REVERSE_DIFF)) { unsigned tmp; - const char *tmp_c; tmp = mode1; mode1 = mode2; mode2 = tmp; - tmp_c = name1; name1 = name2; name2 = tmp_c; + SWAP(name1, name2); } d1 = noindex_filespec(name1, mode1); diff --git a/diff.c b/diff.c index f08cd8e033..9de1ba264f 100644 --- a/diff.c +++ b/diff.c @@ -5118,13 +5118,11 @@ void diff_change(struct diff_options *options, if (DIFF_OPT_TST(options, REVERSE_DIFF)) { unsigned tmp; - const unsigned char *tmp_c; - tmp = old_mode; old_mode = new_mode; new_mode = tmp; - tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c; + SWAP(old_mode, new_mode); + SWAP(old_sha1, new_sha1); tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid; new_sha1_valid = tmp; - tmp = old_dirty_submodule; old_dirty_submodule = new_dirty_submodule; - new_dirty_submodule = tmp; + SWAP(old_dirty_submodule, new_dirty_submodule); } if (options->prefix && diff --git a/graph.c b/graph.c index d4e8519c90..4c722303d2 100644 --- a/graph.c +++ b/graph.c @@ -997,7 +997,6 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct strbuf static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf *sb) { int i; - int *tmp_mapping; short used_horizontal = 0; int horizontal_edge = -1; int horizontal_edge_target = -1; @@ -1132,9 +1131,7 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct strbuf /* * Swap mapping and new_mapping */ - tmp_mapping = graph->mapping; - graph->mapping = graph->new_mapping; - graph->new_mapping = tmp_mapping; + SWAP(graph->mapping, graph->new_mapping); /* * If graph->mapping indicates that all of the branch lines diff --git a/line-range.c b/line-range.c index de4e32f942..323399d16c 100644 --- a/line-range.c +++ b/line-range.c @@ -269,8 +269,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb, return -1; if (*begin && *end && *end < *begin) { - long tmp; - tmp = *end; *end = *begin; *begin = tmp; + SWAP(*end, *begin); } return 0; diff --git a/merge-recursive.c b/merge-recursive.c index d327209443..b7ff1ada3c 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1390,14 +1390,11 @@ static int process_renames(struct merge_options *o, branch1 = o->branch1; branch2 = o->branch2; } else { - struct rename *tmp; renames1 = b_renames; renames2Dst = &a_by_dst; branch1 = o->branch2; branch2 = o->branch1; - tmp = ren2; - ren2 = ren1; - ren1 = tmp; + SWAP(ren2, ren1); } if (ren1->processed) diff --git a/object.c b/object.c index 67d9a9e221..e680d881a4 100644 --- a/object.c +++ b/object.c @@ -104,9 +104,7 @@ struct object *lookup_object(const unsigned char *sha1) * that we do not need to walk the hash table the next * time we look for it. */ - struct object *tmp = obj_hash[i]; - obj_hash[i] = obj_hash[first]; - obj_hash[first] = tmp; + SWAP(obj_hash[i], obj_hash[first]); } return obj; } diff --git a/pack-revindex.c b/pack-revindex.c index 6bc7c94033..1b7ebd8d7e 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -59,7 +59,6 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max) * be a no-op, as everybody lands in the same zero-th bucket. */ for (bits = 0; max >> bits; bits += DIGIT_SIZE) { - struct revindex_entry *swap; unsigned i; memset(pos, 0, BUCKETS * sizeof(*pos)); @@ -97,9 +96,7 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max) * Now "to" contains the most sorted list, so we swap "from" and * "to" for the next iteration. */ - swap = from; - from = to; - to = swap; + SWAP(from, to); } /* diff --git a/prio-queue.c b/prio-queue.c index e4365b00d6..17252d231b 100644 --- a/prio-queue.c +++ b/prio-queue.c @@ -12,9 +12,7 @@ static inline int compare(struct prio_queue *queue, int i, int j) static inline void swap(struct prio_queue *queue, int i, int j) { - struct prio_queue_entry tmp = queue->array[i]; - queue->array[i] = queue->array[j]; - queue->array[j] = tmp; + SWAP(queue->array[i], queue->array[j]); } void prio_queue_reverse(struct prio_queue *queue) diff --git a/strbuf.h b/strbuf.h index 2262b12683..cf1b5409e7 100644 --- a/strbuf.h +++ b/strbuf.h @@ -109,9 +109,7 @@ extern void strbuf_attach(struct strbuf *, void *, size_t, size_t); */ static inline void strbuf_swap(struct strbuf *a, struct strbuf *b) { - struct strbuf tmp = *a; - *a = *b; - *b = tmp; + SWAP(*a, *b); } -- cgit v1.2.3 From 402bf8e19832142de97d84d37c51262d91e5f2db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 28 Jan 2017 22:41:47 +0100 Subject: diff: use SWAP macro Use the macro SWAP to exchange the value of pairs of variables instead of swapping them manually with the help of a temporary variable. The resulting code is shorter and easier to read. The two cases were not transformed by the semantic patch swap.cocci because it's extra careful and handles only cases where the types of all variables are the same -- and here we swap two ints and use an unsigned temporary variable for that. Nevertheless the conversion is safe, as the value range is preserved with and without the patch. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- diff-no-index.c | 3 +-- diff.c | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/diff-no-index.c b/diff-no-index.c index 1ae09894d7..df762fd0f7 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -185,8 +185,7 @@ static int queue_diff(struct diff_options *o, struct diff_filespec *d1, *d2; if (DIFF_OPT_TST(o, REVERSE_DIFF)) { - unsigned tmp; - tmp = mode1; mode1 = mode2; mode2 = tmp; + SWAP(mode1, mode2); SWAP(name1, name2); } diff --git a/diff.c b/diff.c index 9de1ba264f..6c4f3f6b72 100644 --- a/diff.c +++ b/diff.c @@ -5117,11 +5117,9 @@ void diff_change(struct diff_options *options, return; if (DIFF_OPT_TST(options, REVERSE_DIFF)) { - unsigned tmp; SWAP(old_mode, new_mode); SWAP(old_sha1, new_sha1); - tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid; - new_sha1_valid = tmp; + SWAP(old_sha1_valid, new_sha1_valid); SWAP(old_dirty_submodule, new_dirty_submodule); } -- cgit v1.2.3 From 9e2edd66dda418dad751d5eb2e5921e05e57cd30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Sat, 28 Jan 2017 22:42:15 +0100 Subject: graph: use SWAP macro Exchange the values of graph->columns and graph->new_columns using the macro SWAP instead of hand-rolled code. The result is shorter and easier to read. This transformation was not done by the semantic patch swap.cocci because there's an unrelated statement between the second and the last step of the exchange, so it didn't match the expected pattern. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- graph.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/graph.c b/graph.c index 4c722303d2..29b0f51dc5 100644 --- a/graph.c +++ b/graph.c @@ -463,7 +463,6 @@ static void graph_update_width(struct git_graph *graph, static void graph_update_columns(struct git_graph *graph) { struct commit_list *parent; - struct column *tmp_columns; int max_new_columns; int mapping_idx; int i, seen_this, is_commit_in_columns; @@ -476,11 +475,8 @@ static void graph_update_columns(struct git_graph *graph) * We'll re-use the old columns array as storage to compute the new * columns list for the commit after this one. */ - tmp_columns = graph->columns; - graph->columns = graph->new_columns; + SWAP(graph->columns, graph->new_columns); graph->num_columns = graph->num_new_columns; - - graph->new_columns = tmp_columns; graph->num_new_columns = 0; /* -- cgit v1.2.3