threads / discuss / 41994

(unknown)

Subject: (unknown)

## tl;dr

4 messages between Apr 11, 2016 and Apr 12, 2016.

replies: 3people: 4as markdown or json

miwilliams@google.com· Apr 11, 2016, 19:04 UTC · lore
 From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
From: Mike Williams <miwilliams@google.com>
Date: Mon, 11 Apr 2016 14:18:39 -0400
Subject: [PATCH 1/1] wt-status: Remove '!!' from  
wt_status_collect_changed_cb

The wt_status_collect_changed_cb function uses an extraneous double negation (!!) when determining whether or not a submodule has new commits.

Signed-off-by: Mike Williams <miwilliams@google.com>
---
  wt-status.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/wt-status.c b/wt-status.c
index ef74864..b955179 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -431,7 +431,7 @@ static void wt_status_collect_changed_cb(struct  
diff_queue_struct *q,
  			d->worktree_status = p->status;
  		d->dirty_submodule = p->two->dirty_submodule;
  		if (S_ISGITLINK(p->two->mode))
-			d->new_submodule_commits = !!hashcmp(p->one->sha1, p->two->sha1);
+			d->new_submodule_commits = hashcmp(p->one->sha1, p->two->sha1);
  	}
  }

-- 
2.8.0
Jeff King· Apr 11, 2016, 19:13 UTC · re: miwilliams@google.com · lore

Re: your mail

On Mon, Apr 11, 2016 at 07:04:23PM +0000, miwilliams@google.com wrote:
Show 5 quoted lines
> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
> From: Mike Williams <miwilliams@google.com>
> Date: Mon, 11 Apr 2016 14:18:39 -0400
> Subject: [PATCH 1/1] wt-status: Remove '!!' from
> wt_status_collect_changed_cb

These bits (minus the initial "From ..." line) should go into your actual email headers. As it is, your email has no subject line.

Show 6 quoted lines
> The wt_status_collect_changed_cb function uses an extraneous double
> negation (!!) when determining whether or not a submodule has new
> commits.
> [...]
> -			d->new_submodule_commits = !!hashcmp(p->one->sha1, p->two->sha1);
> +			d->new_submodule_commits = hashcmp(p->one->sha1, p->two->sha1);

It's not extraneous. hashcmp() returns 0 for equality, but an arbitrary positive or negative value depending on how the two arguments differ. The assigned "new_submodule_commits" is a bitfield of size 1. So the "!!" is normalizing the value into "0" or "1".

-Peff
Matthieu Moy· Apr 11, 2016, 19:18 UTC · re: miwilliams@google.com · lore

Re: none

miwilliams@google.com writes:
Show 9 quoted lines
> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
> From: Mike Williams <miwilliams@google.com>
> Date: Mon, 11 Apr 2016 14:18:39 -0400
> Subject: [PATCH 1/1] wt-status: Remove '!!' from
> wt_status_collect_changed_cb
>
> The wt_status_collect_changed_cb function uses an extraneous double
> negation (!!)
> when determining whether or not a submodule has new commits.

It's not just a double negation, it's a way to ensure that the value is 0 or 1 (it's a relatively common idiom in C at least in Git's codebase).

new_submodule_commits is a 1-bit bitfield, and you don't want to assign anything other than 1 or 0 (or you'll get modulo 2^n semantics, with n==1).

So the old code is correct and your patch would introduce a bug.
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Stefan Beller· Apr 12, 2016, 04:33 UTC · re: miwilliams@google.com · lore

Re:

On Mon, Apr 11, 2016 at 12:04 PM,  <miwilliams@google.com> wrote:
Show 8 quoted lines
> From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
> From: Mike Williams <miwilliams@google.com>
> Date: Mon, 11 Apr 2016 14:18:39 -0400
> Subject: [PATCH 1/1] wt-status: Remove '!!' from
> wt_status_collect_changed_cb
>
> The wt_status_collect_changed_cb function uses an extraneous double negation
> (!!)
How is an !! errornous?

It serves the purpose to map an integer value(-1,0,1,2,3,4) to a boolean (0,1, or a real bit in a bit field).

Show 30 quoted lines
> when determining whether or not a submodule has new commits.
>
> Signed-off-by: Mike Williams <miwilliams@google.com>
> ---
>  wt-status.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/wt-status.c b/wt-status.c
> index ef74864..b955179 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -431,7 +431,7 @@ static void wt_status_collect_changed_cb(struct
> diff_queue_struct *q,
>                         d->worktree_status = p->status;
>                 d->dirty_submodule = p->two->dirty_submodule;
>                 if (S_ISGITLINK(p->two->mode))
> -                       d->new_submodule_commits = !!hashcmp(p->one->sha1,
> p->two->sha1);
> +                       d->new_submodule_commits = hashcmp(p->one->sha1,
> p->two->sha1);
>         }
>  }
>
> --
> 2.8.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

← back to recent threads