{"thread":{"id":"47613","subject":"[PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes","startedAt":"2018-01-15T17:11:32Z","lastAt":"2018-01-22T23:34:32Z","messageCount":11,"participants":["René Scharfe","Derrick Stolee","SZEDER Gábor","Lars Schneider","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"336624","messageId":"422ae05b-6541-38f3-7638-4bee1b766d91@web.de","threadId":"47613","inReplyTo":null,"subject":"[PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-01-15T17:10:32Z","receivedAt":"2018-01-15T17:11:32Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Call strbuf_add_unique_abbrev() to add an abbreviated hash to a strbuf\ninstead of taking a detour through find_unique_abbrev() and its static\nbuffer. This is shorter and a bit more efficient.\n\nPatch generated by Coccinelle (and contrib/coccinelle/strbuf.cocci).\n\nSigned-off-by: Rene Scharfe \n---\nThe changed line was added by 4dbc59a4cc (builtin/describe.c: factor\nout describe_commit).\n\n\"make coccicheck\" doesn't propose any other changes for current master.\n\n builtin/describe.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex 3b0b204b1e..21e37f5dae 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -380,7 +380,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)\n \tif (!match_cnt) {\n \t\tstruct object_id *cmit_oid = &cmit->object.oid;\n \t\tif (always) {\n-\t\t\tstrbuf_addstr(dst, find_unique_abbrev(cmit_oid->hash, abbrev));\n+\t\t\tstrbuf_add_unique_abbrev(dst, cmit_oid->hash, abbrev);\n \t\t\tif (suffix)\n \t\t\t\tstrbuf_addstr(dst, suffix);\n \t\t\treturn;\n-- \n2.15.1\n"},{"id":"336657","messageId":"6f91c34e-3ab0-30fc-4678-4424ac350cd9@gmail.com","threadId":"47613","inReplyTo":"422ae05b-6541-38f3-7638-4bee1b766d91@web.de","subject":"Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-01-16T13:48:35Z","receivedAt":"2018-01-16T13:48:44Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 1/15/2018 12:10 PM, René Scharfe wrote:\n> Call strbuf_add_unique_abbrev() to add an abbreviated hash to a strbuf\n> instead of taking a detour through find_unique_abbrev() and its static\n> buffer. This is shorter and a bit more efficient.\n>\n> Patch generated by Coccinelle (and contrib/coccinelle/strbuf.cocci).\n>\n> Signed-off-by: Rene Scharfe \n> ---\n> The changed line was added by 4dbc59a4cc (builtin/describe.c: factor\n> out describe_commit).\n>\n> \"make coccicheck\" doesn't propose any other changes for current master.\n>\n> builtin/describe.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/describe.c b/builtin/describe.c\n> index 3b0b204b1e..21e37f5dae 100644\n> --- a/builtin/describe.c\n> +++ b/builtin/describe.c\n> @@ -380,7 +380,7 @@ static void describe_commit(struct object_id *oid, struct strbuf *dst)\n> \tif (!match_cnt) {\n> \t\tstruct object_id *cmit_oid = &cmit->object.oid;\n> \t\tif (always) {\n> -\t\t\tstrbuf_addstr(dst, find_unique_abbrev(cmit_oid->hash, abbrev));\n> +\t\t\tstrbuf_add_unique_abbrev(dst, cmit_oid->hash, abbrev);\n> \t\t\tif (suffix)\n> \t\t\t\tstrbuf_addstr(dst, suffix);\n> \t\t\treturn;\n\nRené,\n\nThanks for this cleanup! I just learned about strbuf_add_unique_abbrev() \nand like that it uses the reentrant find_unique_abbrev_r() instead.\n\nLooks good to me.\n-Stolee\n\n"},{"id":"336660","messageId":"20180116171130.16568-1-szeder.dev@gmail.com","threadId":"47613","inReplyTo":"422ae05b-6541-38f3-7638-4bee1b766d91@web.de","subject":"Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-01-16T17:11:30Z","receivedAt":"2018-01-16T17:11:57Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"> Patch generated by Coccinelle (and contrib/coccinelle/strbuf.cocci).\n\nInteresting.\n\nThe static analysis build job on Travis CI runs 'make coccicheck', so\nit should have caught this. However, I've looked at more build job\nresults than I could count while working on some Travis CI related\npatches in the last few weeks, but I've never noticed Coccinelle\nproposing anything.\n\nNow I see that Coccinelle's, and in turn 'make coccicheck's exit code\nonly indicates that Coccinelle managed to finish without any errors\n(e.g. no wrong --options, missing files or whatnot), but it has\nnothing to do with whether it found something to transform or not. To\nfind out the latter, we have to check the resulting\n'contrib/coccinelle/*.cocci.patch' files or look closer at 'make\ncoccicheck's standard output. If any of those '*.cocci.patch' files\nare not empty and make's output contains lines like:\n\n SPATCH result: contrib/coccinelle/strbuf.cocci.patch\n\nthen Coccinelle found something.\n\nWell, OK, I think that might be an acceptable behavior for developers\nrunning 'make coccicheck' themselves, but totally unsuitable for\nautomated Travis CI build jobs, because it doesn't draw our attention\nto Coccinelle's findings. And the only means to draw attention in\nan automated setting is to fail the buid job.\n\nGood, I quickly whipped up a patch to fail the build job if any of the\nresulting '*.cocci.patch' files are not empty and also to include\ntheir content in the trace log, to see how that would work out, and...\n\n> \"make coccicheck\" doesn't propose any other changes for current master.\n\n... and found that 'make coccicheck' on Travis CI _does_ propose\nfurther changes for current master. You can find the build job's\noutput including those proposed changes here:\n\n https://travis-ci.org/szeder/git/jobs/329296813#L586\n\nThe proposed changes coming from 'array.cocci' are replacing memmove()\ncalls with MOVE_ARRAY(). After a (very) cursory look they all seem to\nmake sense to me; at least after applying those changes Git can be\nbuilt and the test suite still succeeds.\n\nUnfortunately, most of the changes coming from 'strbuf.cocci' don't\nmake any sense, they appear to be the mis-application of the \"use\nstrbuf_addstr() instead of strbuf_addf() to add a single string\" rule:\n\n - strbuf_addf(&sb_repo, \"%d\", counter);\n + strbuf_addstr(&sb_repo, counter);\n\nIt seems that those rules need some refinement, but I have no idea\nabout Coccinelle and this is not the time for me to dig deeper.\n\nWhat makes all this weird is that running 'make coccicheck' on my own\nmachine doesn't produce any of these additional proposed changes, just\nlike at René's. Can it be related to differing Coccinelle versions?\nTravis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.\n\n\nGábor\n\n"},{"id":"336832","messageId":"7390fd9c-5969-61f1-86b8-6971eaf1432b@web.de","threadId":"47613","inReplyTo":"20180116171130.16568-1-szeder.dev@gmail.com","subject":"Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-01-18T21:40:09Z","receivedAt":"2018-01-18T21:40:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 16.01.2018 um 18:11 schrieb SZEDER Gábor:\n> Unfortunately, most of the changes coming from 'strbuf.cocci' don't\n> make any sense, they appear to be the mis-application of the \"use\n> strbuf_addstr() instead of strbuf_addf() to add a single string\" rule:\n> \n> - strbuf_addf(&sb_repo, \"%d\", counter);\n> + strbuf_addstr(&sb_repo, counter);\n> \n> It seems that those rules need some refinement, but I have no idea\n> about Coccinelle and this is not the time for me to dig deeper.\n> \n> What makes all this weird is that running 'make coccicheck' on my own\n> machine doesn't produce any of these additional proposed changes, just\n> like at René's. Can it be related to differing Coccinelle versions?\n> Travis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.\n\nThe version difference may explain it, but I couldn't find a matching\nbugfix in http://coccinelle.lip6.fr/distrib/changes.html when I just\nskimmed it. I wonder if the following patch could make a difference:\n\n---\n contrib/coccinelle/strbuf.cocci | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci\nindex 1d580e49b0..6fe8727421 100644\n--- a/contrib/coccinelle/strbuf.cocci\n+++ b/contrib/coccinelle/strbuf.cocci\n@@ -29,8 +29,9 @@ cocci.include_match(\"%\" not in fmt)\n \n @@\n expression E1, E2;\n+format F =~ \"s\";\n @@\n-- strbuf_addf(E1, \"%s\", E2);\n+- strbuf_addf(E1, \"%@F@\", E2);\n + strbuf_addstr(E1, E2);\n \n @@\n-- \n2.16.0\n"},{"id":"336844","messageId":"CAM0VKj=eL5ZnnwKrxqtDYE+ijqJvcU-4bJNhjurvdn+Bpv7gUQ@mail.gmail.com","threadId":"47613","inReplyTo":"7390fd9c-5969-61f1-86b8-6971eaf1432b@web.de","subject":"Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-01-18T22:40:17Z","receivedAt":"2018-01-18T22:40:22Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Jan 18, 2018 at 10:40 PM, René Scharfe wrote:\n> Am 16.01.2018 um 18:11 schrieb SZEDER Gábor:\n>> Unfortunately, most of the changes coming from 'strbuf.cocci' don't\n>> make any sense, they appear to be the mis-application of the \"use\n>> strbuf_addstr() instead of strbuf_addf() to add a single string\" rule:\n>>\n>> - strbuf_addf(&sb_repo, \"%d\", counter);\n>> + strbuf_addstr(&sb_repo, counter);\n>>\n>> It seems that those rules need some refinement, but I have no idea\n>> about Coccinelle and this is not the time for me to dig deeper.\n>>\n>> What makes all this weird is that running 'make coccicheck' on my own\n>> machine doesn't produce any of these additional proposed changes, just\n>> like at René's. Can it be related to differing Coccinelle versions?\n>> Travis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.\n>\n> The version difference may explain it, but I couldn't find a matching\n> bugfix in http://coccinelle.lip6.fr/distrib/changes.html when I just\n> skimmed it. I wonder if the following patch could make a difference:\n\nYes, it does, now all those nonsense suggestions are gone on Travis CI.\n\n https://travis-ci.org/szeder/git/jobs/330572425#L713\n\nThose \"memmove() -> MOVE_ARRAY\" suggestions are still there, of course.\n\n> ---\n> contrib/coccinelle/strbuf.cocci | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci\n> index 1d580e49b0..6fe8727421 100644\n> --- a/contrib/coccinelle/strbuf.cocci\n> +++ b/contrib/coccinelle/strbuf.cocci\n> @@ -29,8 +29,9 @@ cocci.include_match(\"%\" not in fmt)\n>\n> @@\n> expression E1, E2;\n> +format F =~ \"s\";\n> @@\n> -- strbuf_addf(E1, \"%s\", E2);\n> +- strbuf_addf(E1, \"%@F@\", E2);\n> + strbuf_addstr(E1, E2);\n>\n> @@\n> --\n> 2.16.0\n"},{"id":"336848","messageId":"89BB7BC3-1EFB-414F-8849-B520AD4543E6@gmail.com","threadId":"47613","inReplyTo":"CAM0VKj=eL5ZnnwKrxqtDYE+ijqJvcU-4bJNhjurvdn+Bpv7gUQ@mail.gmail.com","subject":"Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2018-01-18T23:02:52Z","receivedAt":"2018-01-18T23:03:00Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 18 Jan 2018, at 23:40, SZEDER Gábor wrote:\n> \n> On Thu, Jan 18, 2018 at 10:40 PM, René Scharfe wrote:\n>> Am 16.01.2018 um 18:11 schrieb SZEDER Gábor:\n>>> Unfortunately, most of the changes coming from 'strbuf.cocci' don't\n>>> make any sense, they appear to be the mis-application of the \"use\n>>> strbuf_addstr() instead of strbuf_addf() to add a single string\" rule:\n>>> \n>>> - strbuf_addf(&sb_repo, \"%d\", counter);\n>>> + strbuf_addstr(&sb_repo, counter);\n>>> \n>>> It seems that those rules need some refinement, but I have no idea\n>>> about Coccinelle and this is not the time for me to dig deeper.\n>>> \n>>> What makes all this weird is that running 'make coccicheck' on my own\n>>> machine doesn't produce any of these additional proposed changes, just\n>>> like at René's. Can it be related to differing Coccinelle versions?\n>>> Travis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.\n>> \n>> The version difference may explain it, but I couldn't find a matching\n>> bugfix in http://coccinelle.lip6.fr/distrib/changes.html when I just\n>> skimmed it. I wonder if the following patch could make a difference:\n> \n> Yes, it does, now all those nonsense suggestions are gone on Travis CI.\n> \n> https://travis-ci.org/szeder/git/jobs/330572425#L713\n> \n> Those \"memmove() -> MOVE_ARRAY\" suggestions are still there, of course.\n\nNice!\n\nTravis CI runs Ubuntu Trusty as current base image with Coccinelle\nversion \"1.0.0~rc19.deb-3\": https://packages.ubuntu.com/trusty/coccinelle\n\nI was experimenting with a docker container that has the latest stable\nversion of Coccinelle installed (\"1.0.4.deb-2\"). We could run this\ndocker container inside of the Travis CI job similar to the 32-bit job.\nBut with René's changes that doesn't seem to be necessary anymore!\n\n- Lars\n\n\nPS: Thanks a lot Gábor for recognizing and fixing the invalid\n Travis CI Coccinelle build!"},{"id":"336911","messageId":"d81743e5-d5ba-a565-23f7-072007493499@web.de","threadId":"47613","inReplyTo":"CAM0VKj=eL5ZnnwKrxqtDYE+ijqJvcU-4bJNhjurvdn+Bpv7gUQ@mail.gmail.com","subject":"Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-01-19T17:53:40Z","receivedAt":"2018-01-19T17:53:52Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 18.01.2018 um 23:40 schrieb SZEDER Gábor:\n> On Thu, Jan 18, 2018 at 10:40 PM, René Scharfe wrote:\n>> Am 16.01.2018 um 18:11 schrieb SZEDER Gábor:\n>>> Unfortunately, most of the changes coming from 'strbuf.cocci' don't\n>>> make any sense, they appear to be the mis-application of the \"use\n>>> strbuf_addstr() instead of strbuf_addf() to add a single string\" rule:\n>>>\n>>> - strbuf_addf(&sb_repo, \"%d\", counter);\n>>> + strbuf_addstr(&sb_repo, counter);\n>>>\n>>> It seems that those rules need some refinement, but I have no idea\n>>> about Coccinelle and this is not the time for me to dig deeper.\n>>>\n>>> What makes all this weird is that running 'make coccicheck' on my own\n>>> machine doesn't produce any of these additional proposed changes, just\n>>> like at René's. Can it be related to differing Coccinelle versions?\n>>> Travis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.\n>>\n>> The version difference may explain it, but I couldn't find a matching\n>> bugfix in http://coccinelle.lip6.fr/distrib/changes.html when I just\n>> skimmed it. I wonder if the following patch could make a difference:\n> \n> Yes, it does, now all those nonsense suggestions are gone on Travis CI.\n\nI would have expected matching a literal \"%s\" to be easier than\ndissecting that (admittedly simple) format string, but if it all works\nout fine then I'm not complaining. :) Sent the patch again properly.\n\n> https://travis-ci.org/szeder/git/jobs/330572425#L713\n> \n> Those \"memmove() -> MOVE_ARRAY\" suggestions are still there, of course.\n\nThey look valid and nice to have in that report. I wonder why we don't\nget them locally, though. Are you going to submit them as a patch?\n\n(NB: The patches generated by coccicheck apply with \"patch -p0\", unlike\n those generated by git diff and friends.)\n\nThanks,\nRené\n"},{"id":"337093","messageId":"20180122175009.20178-1-szeder.dev@gmail.com","threadId":"47613","inReplyTo":"d81743e5-d5ba-a565-23f7-072007493499@web.de","subject":"[PATCH] Use MOVE_ARRAY","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-01-22T17:50:09Z","receivedAt":"2018-01-22T17:50:27Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"Use the helper macro MOVE_ARRAY to move arrays. This is shorter and\nsafer, as it automatically infers the size of elements.\n\nPatch generated by Coccinelle and contrib/coccinelle/array.cocci in\nTravis CI's static analysis build job.\n\nSigned-off-by: SZEDER Gábor \n---\n cache-tree.c | 5 ++---\n commit.c | 6 ++----\n diffcore-rename.c | 8 ++++----\n dir.c | 4 ++--\n parse-options.c | 2 +-\n read-cache.c | 5 ++---\n refs/ref-cache.c | 6 ++----\n replace_object.c | 6 ++----\n rerere.c | 4 ++--\n 9 files changed, 19 insertions(+), 27 deletions(-)\n\ndiff --git a/cache-tree.c b/cache-tree.c\nindex e03e72c34a..18946aa458 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -84,9 +84,8 @@ static struct cache_tree_sub *find_subtree(struct cache_tree *it,\n \tdown->namelen = pathlen;\n \n \tif (pos < it->subtree_nr)\n-\t\tmemmove(it->down + pos + 1,\n-\t\t\tit->down + pos,\n-\t\t\tsizeof(down) * (it->subtree_nr - pos - 1));\n+\t\tMOVE_ARRAY(it->down + pos + 1, it->down + pos,\n+\t\t\t it->subtree_nr - pos - 1);\n \tit->down[pos] = down;\n \treturn down;\n }\ndiff --git a/commit.c b/commit.c\nindex cab8d4455b..7d0080180d 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -126,10 +126,8 @@ int register_commit_graft(struct commit_graft *graft, int ignore_dups)\n \tALLOC_GROW(commit_graft, commit_graft_nr + 1, commit_graft_alloc);\n \tcommit_graft_nr++;\n \tif (pos < commit_graft_nr)\n-\t\tmemmove(commit_graft + pos + 1,\n-\t\t\tcommit_graft + pos,\n-\t\t\t(commit_graft_nr - pos - 1) *\n-\t\t\tsizeof(*commit_graft));\n+\t\tMOVE_ARRAY(commit_graft + pos + 1, commit_graft + pos,\n+\t\t\t commit_graft_nr - pos - 1);\n \tcommit_graft[pos] = graft;\n \treturn 0;\n }\ndiff --git a/diffcore-rename.c b/diffcore-rename.c\nindex 245e999fe5..888a4b0189 100644\n--- a/diffcore-rename.c\n+++ b/diffcore-rename.c\n@@ -57,8 +57,8 @@ static int add_rename_dst(struct diff_filespec *two)\n \tALLOC_GROW(rename_dst, rename_dst_nr + 1, rename_dst_alloc);\n \trename_dst_nr++;\n \tif (first < rename_dst_nr)\n-\t\tmemmove(rename_dst + first + 1, rename_dst + first,\n-\t\t\t(rename_dst_nr - first - 1) * sizeof(*rename_dst));\n+\t\tMOVE_ARRAY(rename_dst + first + 1, rename_dst + first,\n+\t\t\t rename_dst_nr - first - 1);\n \trename_dst[first].two = alloc_filespec(two->path);\n \tfill_filespec(rename_dst[first].two, &two->oid, two->oid_valid,\n \t\t two->mode);\n@@ -98,8 +98,8 @@ static struct diff_rename_src *register_rename_src(struct diff_filepair *p)\n \tALLOC_GROW(rename_src, rename_src_nr + 1, rename_src_alloc);\n \trename_src_nr++;\n \tif (first < rename_src_nr)\n-\t\tmemmove(rename_src + first + 1, rename_src + first,\n-\t\t\t(rename_src_nr - first - 1) * sizeof(*rename_src));\n+\t\tMOVE_ARRAY(rename_src + first + 1, rename_src + first,\n+\t\t\t rename_src_nr - first - 1);\n \trename_src[first].p = p;\n \trename_src[first].score = score;\n \treturn &(rename_src[first]);\ndiff --git a/dir.c b/dir.c\nindex 7c4b45e30e..ce6e50d2a2 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -747,8 +747,8 @@ static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,\n \tFLEX_ALLOC_MEM(d, name, name, len);\n \n \tALLOC_GROW(dir->dirs, dir->dirs_nr + 1, dir->dirs_alloc);\n-\tmemmove(dir->dirs + first + 1, dir->dirs + first,\n-\t\t(dir->dirs_nr - first) * sizeof(*dir->dirs));\n+\tMOVE_ARRAY(dir->dirs + first + 1, dir->dirs + first,\n+\t\t dir->dirs_nr - first);\n \tdir->dirs_nr++;\n \tdir->dirs[first] = d;\n \treturn d;\ndiff --git a/parse-options.c b/parse-options.c\nindex fca7159646..d02eb8b015 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -525,7 +525,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,\n \n int parse_options_end(struct parse_opt_ctx_t *ctx)\n {\n-\tmemmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));\n+\tMOVE_ARRAY(ctx->out + ctx->cpidx, ctx->argv, ctx->argc);\n \tctx->out[ctx->cpidx + ctx->argc] = NULL;\n \treturn ctx->cpidx + ctx->argc;\n }\ndiff --git a/read-cache.c b/read-cache.c\nindex 2eb81a66b9..2e8c85c686 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -1217,9 +1217,8 @@ int add_index_entry(struct index_state *istate, struct cache_entry *ce, int opti\n \t/* Add it in.. */\n \tistate->cache_nr++;\n \tif (istate->cache_nr > pos + 1)\n-\t\tmemmove(istate->cache + pos + 1,\n-\t\t\tistate->cache + pos,\n-\t\t\t(istate->cache_nr - pos - 1) * sizeof(ce));\n+\t\tMOVE_ARRAY(istate->cache + pos + 1, istate->cache + pos,\n+\t\t\t istate->cache_nr - pos - 1);\n \tset_index_entry(istate, pos, ce);\n \tistate->cache_changed |= CE_ENTRY_ADDED;\n \treturn 0;\ndiff --git a/refs/ref-cache.c b/refs/ref-cache.c\nindex 82c1cf90a7..e90bd3e727 100644\n--- a/refs/ref-cache.c\n+++ b/refs/ref-cache.c\n@@ -238,10 +238,8 @@ int remove_entry_from_dir(struct ref_dir *dir, const char *refname)\n \t\treturn -1;\n \tentry = dir->entries[entry_index];\n \n-\tmemmove(&dir->entries[entry_index],\n-\t\t&dir->entries[entry_index + 1],\n-\t\t(dir->nr - entry_index - 1) * sizeof(*dir->entries)\n-\t\t);\n+\tMOVE_ARRAY(&dir->entries[entry_index],\n+\t\t &dir->entries[entry_index + 1], dir->nr - entry_index - 1);\n \tdir->nr--;\n \tif (dir->sorted > entry_index)\n \t\tdir->sorted--;\ndiff --git a/replace_object.c b/replace_object.c\nindex f0b39f06d5..3e49965d05 100644\n--- a/replace_object.c\n+++ b/replace_object.c\n@@ -44,10 +44,8 @@ static int register_replace_object(struct replace_object *replace,\n \tALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc);\n \treplace_object_nr++;\n \tif (pos < replace_object_nr)\n-\t\tmemmove(replace_object + pos + 1,\n-\t\t\treplace_object + pos,\n-\t\t\t(replace_object_nr - pos - 1) *\n-\t\t\tsizeof(*replace_object));\n+\t\tMOVE_ARRAY(replace_object + pos + 1, replace_object + pos,\n+\t\t\t replace_object_nr - pos - 1);\n \treplace_object[pos] = replace;\n \treturn 0;\n }\ndiff --git a/rerere.c b/rerere.c\nindex 1ce440f4bb..79203c6c1e 100644\n--- a/rerere.c\n+++ b/rerere.c\n@@ -159,8 +159,8 @@ static struct rerere_dir *find_rerere_dir(const char *hex)\n \t\tALLOC_GROW(rerere_dir, rerere_dir_nr + 1, rerere_dir_alloc);\n \t\t/* ... and add it in. */\n \t\trerere_dir_nr++;\n-\t\tmemmove(rerere_dir + pos + 1, rerere_dir + pos,\n-\t\t\t(rerere_dir_nr - pos - 1) * sizeof(*rerere_dir));\n+\t\tMOVE_ARRAY(rerere_dir + pos + 1, rerere_dir + pos,\n+\t\t\t rerere_dir_nr - pos - 1);\n \t\trerere_dir[pos] = rr_dir;\n \t\tscan_rerere_dir(rr_dir);\n \t}\n-- \n2.16.1.80.gc0eec9753d\n\n"},{"id":"337112","messageId":"20180122224410.GA21604@sigill.intra.peff.net","threadId":"47613","inReplyTo":"20180122175009.20178-1-szeder.dev@gmail.com","subject":"Re: [PATCH] Use MOVE_ARRAY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-22T22:44:10Z","receivedAt":"2018-01-22T22:44:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 22, 2018 at 06:50:09PM +0100, SZEDER Gábor wrote:\n\n> Use the helper macro MOVE_ARRAY to move arrays. This is shorter and\n> safer, as it automatically infers the size of elements.\n> \n> Patch generated by Coccinelle and contrib/coccinelle/array.cocci in\n> Travis CI's static analysis build job.\n\nSeems pretty straightforward. One thing I did notice...\n\n> diff --git a/cache-tree.c b/cache-tree.c\n> index e03e72c34a..18946aa458 100644\n> --- a/cache-tree.c\n> +++ b/cache-tree.c\n> @@ -84,9 +84,8 @@ static struct cache_tree_sub *find_subtree(struct cache_tree *it,\n> \tdown->namelen = pathlen;\n> \n> \tif (pos < it->subtree_nr)\n> -\t\tmemmove(it->down + pos + 1,\n> -\t\t\tit->down + pos,\n> -\t\t\tsizeof(down) * (it->subtree_nr - pos - 1));\n> +\t\tMOVE_ARRAY(it->down + pos + 1, it->down + pos,\n> +\t\t\t it->subtree_nr - pos - 1);\n\nMost of these are \"shift part of the array\". I wonder if it would make\nsense to encapsulate that pattern in a helper, like:\n\n #define SHIFT_ARRAY(a, nr, pos, slots) \\\n MOVE_ARRAY(a + pos + slots, a + pos, nr - pos - slots)\n ...\n SHIFT_ARRAY(it->down, it->subtree_nr, pos, 1);\n\nI'm not sure if that's more readable because it describes a higher-level\noperation, or if it's less because it adds yet another non-standard\nhelper for the reader to learn.\n\n-Peff\n"},{"id":"337120","messageId":"xmqq372xjwzg.fsf@gitster.mtv.corp.google.com","threadId":"47613","inReplyTo":"20180122224410.GA21604@sigill.intra.peff.net","subject":"Re: [PATCH] Use MOVE_ARRAY","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-22T23:26:59Z","receivedAt":"2018-01-22T23:27:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Most of these are \"shift part of the array\". I wonder if it would make\n> sense to encapsulate that pattern in a helper, like:\n>\n> #define SHIFT_ARRAY(a, nr, pos, slots) \\\n> MOVE_ARRAY(a + pos + slots, a + pos, nr - pos - slots)\n> ...\n> SHIFT_ARRAY(it->down, it->subtree_nr, pos, 1);\n\nExactly my thought when I was queuing it, but I was wondering about\nthis more from \"can we use the higher level abstraction for reducing\nerrors?\" point of view. If we are shifting an array by 3 slots to\nthe right, we should at least have enough slots allocated to hold\nthem (i.e. a->nr - a->alloc must be 3 or more). But after realizing\nthat the level these macros operate at is still a bit too low to do\nsomething like that, I quickly lost interest ;-)\n\n> I'm not sure if that's more readable because it describes a higher-level\n> operation, or if it's less because it adds yet another non-standard\n> helper for the reader to learn.\n\nYeah, conflicting goals. It indeed is easier to see what is going\non when reading the code, once the reader gets used to them.\n\nThanks.\n"},{"id":"337122","messageId":"20180122233426.GA27144@sigill.intra.peff.net","threadId":"47613","inReplyTo":"xmqq372xjwzg.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Use MOVE_ARRAY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-22T23:34:26Z","receivedAt":"2018-01-22T23:34:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 22, 2018 at 03:26:59PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Most of these are \"shift part of the array\". I wonder if it would make\n> > sense to encapsulate that pattern in a helper, like:\n> >\n> > #define SHIFT_ARRAY(a, nr, pos, slots) \\\n> > MOVE_ARRAY(a + pos + slots, a + pos, nr - pos - slots)\n> > ...\n> > SHIFT_ARRAY(it->down, it->subtree_nr, pos, 1);\n> \n> Exactly my thought when I was queuing it, but I was wondering about\n> this more from \"can we use the higher level abstraction for reducing\n> errors?\" point of view. If we are shifting an array by 3 slots to\n> the right, we should at least have enough slots allocated to hold\n> them (i.e. a->nr - a->alloc must be 3 or more). But after realizing\n> that the level these macros operate at is still a bit too low to do\n> something like that, I quickly lost interest ;-)\n\nYeah, you'd need to know the \"alloc\" number to right-shift correctly,\nsince by definition we're pushing off the end of a->nr. Left-shifts just\nneed to make sure they don't go past \"0\", which we can do here, but I'd\nthink they're pretty uncommon.\n\nThe right macro level may actually be something more like \"make room for\nN items at pos\". E.g.:\n\n #define CREATE_ARRAY_HOLE(a, nr, alloc, pos, slots) do { \\\n ALLOC_GROW(a, nr + slots, alloc);\n\t MOVE_ARRAY(a + pos + slots, a + pos, nr - pos - slots);\n } while (0)\n\nbut I didn't investigate the surrounding code. And it surely would need\na catchier name. ;)\n\n-Peff\n"}]}