Re: [PATCH 3/8] mv: flatten error handling code block
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 8, 2014, 17:54 UTC
- Message-ID
- <xmqqbnrukgua.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1407503462-32632-4-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 55 quoted lines
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> builtin/mv.c | 35 +++++++++++++++++------------------
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/builtin/mv.c b/builtin/mv.c
> index a7e02c0..5c6f58f 100644
> --- a/builtin/mv.c
> +++ b/builtin/mv.c
> @@ -58,6 +58,11 @@ static const char *add_slash(const char *path)
> return path;
> }
>
> +static void move_up_one(void *p, int nmemb, int size)
> +{
> + memmove(p, (char*)p + size, nmemb * size);
> +}
> +
> static struct lock_file lock_file;
> #define SUBMODULE_WITH_GITDIR ((const char *)1)
>
> @@ -224,24 +229,18 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
> else
> string_list_insert(&src_for_dst, dst);
>
> - if (bad) {
> - if (ignore_errors) {
> - if (--argc > 0) {
> - memmove(source + i, source + i + 1,
> - (argc - i) * sizeof(char *));
> - memmove(destination + i,
> - destination + i + 1,
> - (argc - i) * sizeof(char *));
> - memmove(modes + i, modes + i + 1,
> - (argc - i) * sizeof(enum update_mode));
> - memmove(submodule_gitfile + i,
> - submodule_gitfile + i + 1,
> - (argc - i) * sizeof(char *));
> - i--;
> - }
> - } else
> - die (_("%s, source=%s, destination=%s"),
> - bad, src, dst);
> + if (!bad)
> + continue;
> + if (!ignore_errors)
> + die (_("%s, source=%s, destination=%s"),
> + bad, src, dst);
> + if (--argc > 0) {
> + int n = argc - i;
> + move_up_one(source + i, n, sizeof(*source));
> + move_up_one(destination + i, n, sizeof(*destination));
> + move_up_one(modes + i, n, sizeof(*modes));
> + move_up_one(submodule_gitfile + i, n, sizeof(*submodule_gitfile));
> + i--;The resulting end-of-loop code structure certainly looks a lot better, even if the original memmove()s were left inline without the helper.
The helper itself however looks a bit half-hearted. It may be more appropriate to go one step further to have a macro whose use looks like this, perhaps, to avoid the last remaining repetition?
MOVE_UP_BY_ONE(source, i, n);
> } > }