public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: "Nguyễn Thái Ngọc Duy" <pclouds@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 3/8] mv: flatten error handling code block
Date: Fri, 08 Aug 2014 10:54:05 -0700	[thread overview]
Message-ID: <xmqqbnrukgua.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1407503462-32632-4-git-send-email-pclouds@gmail.com> ("Nguyễn	Thái Ngọc Duy"'s message of "Fri, 8 Aug 2014 20:10:57 +0700")

Nguyễn Thái Ngọc Duy  <pclouds@gmail•com> writes:

> 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);


>  		}
>  	}

  reply	other threads:[~2014-08-08 17:54 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-08 13:10 [PATCH 0/8] builtin/mv.c cleanup Nguyễn Thái Ngọc Duy
2014-08-08 13:10 ` [PATCH 1/8] mv: mark strings for translations Nguyễn Thái Ngọc Duy
2014-08-08 13:10 ` [PATCH 2/8] mv: no "Huh?" to the user Nguyễn Thái Ngọc Duy
2014-08-08 17:48   ` Junio C Hamano
2014-08-08 13:10 ` [PATCH 3/8] mv: flatten error handling code block Nguyễn Thái Ngọc Duy
2014-08-08 17:54   ` Junio C Hamano [this message]
2014-08-08 13:10 ` [PATCH 4/8] mv: split submodule move preparation code out Nguyễn Thái Ngọc Duy
2014-08-08 13:10 ` [PATCH 5/8] mv: remove an "if" that's always true Nguyễn Thái Ngọc Duy
2014-08-08 13:11 ` [PATCH 6/8] mv: move index search code out Nguyễn Thái Ngọc Duy
2014-08-08 13:11 ` [PATCH 7/8] mv: unindent one level for directory move code Nguyễn Thái Ngọc Duy
2014-08-08 13:11 ` [PATCH 8/8] mv: combine two if(s) Nguyễn Thái Ngọc Duy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqbnrukgua.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=pclouds@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox