| Message ID | xmqq4jzpu4xp.fsf_-_@gitster.g (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | eee227ad8e759c0e7f625de3ee4458f85e4c9539 |
| Headers | show |
| Series | [v3] builtin/mv.c: use the MOVE_ARRAY() macro instead of memmove() | expand |
On 7/9/2022 9:33 PM, Junio C Hamano wrote: > The variables 'source', 'destination', and 'submodule_gitfile' are > all of type "const char **", and an element of such an array is of > "type const char *", but these memmove() calls were written as if > these variables are of type "char **". > > Once these memmove() calls are fixed to use the correct type to > compute the number of bytes to be moved, e.g. > > - memmove(source + i, source + i + 1, n * sizeof(char *)); > + memmove(source + i, source + i + 1, n * sizeof(const char *)); > > existing contrib/coccinelle/array.cocci rules can recognize them as > candidates for turning into MOVE_ARRAY(). > > While at it, use CALLOC_ARRAY() instead of xcalloc() to allocate the > modes[] array that is involved in the change. I'm super late in noticing this, but these changes will fix the static-analysis build in the CI build, so the sooner this can make it to master the better. Thanks! -Stolee
diff --git a/builtin/mv.c b/builtin/mv.c index 83a465ba83..859fa5023f 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -148,7 +148,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix) die(_("index file corrupt")); source = internal_prefix_pathspec(prefix, argv, argc, 0); - modes = xcalloc(argc, sizeof(enum update_mode)); + CALLOC_ARRAY(modes, argc); + /* * Keep trailing slash, needed to let * "git mv file no-such-dir/" error out, except in the case @@ -282,14 +283,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix) remove_entry: if (--argc > 0) { int n = argc - i; - memmove(source + i, source + i + 1, - n * sizeof(char *)); - memmove(destination + i, destination + i + 1, - n * sizeof(char *)); - memmove(modes + i, modes + i + 1, - n * sizeof(enum update_mode)); - memmove(submodule_gitfile + i, submodule_gitfile + i + 1, - n * sizeof(char *)); + MOVE_ARRAY(source + i, source + i + 1, n); + MOVE_ARRAY(destination + i, destination + i + 1, n); + MOVE_ARRAY(modes + i, modes + i + 1, n); + MOVE_ARRAY(submodule_gitfile + i, + submodule_gitfile + i + 1, n); i--; } }
The variables 'source', 'destination', and 'submodule_gitfile' are all of type "const char **", and an element of such an array is of "type const char *", but these memmove() calls were written as if these variables are of type "char **". Once these memmove() calls are fixed to use the correct type to compute the number of bytes to be moved, e.g. - memmove(source + i, source + i + 1, n * sizeof(char *)); + memmove(source + i, source + i + 1, n * sizeof(const char *)); existing contrib/coccinelle/array.cocci rules can recognize them as candidates for turning into MOVE_ARRAY(). While at it, use CALLOC_ARRAY() instead of xcalloc() to allocate the modes[] array that is involved in the change. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * The first hunk is new; with this additional rewrite from xcalloc() to CALLOC_ARRAY(), the static-analysis job on 'seen' starts passing (cf. https://github.com/git/git/runs/7267146111) I think this is now good enough to merge down to 'next' and to 'master'. builtin/mv.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)