Re: [PATCH MISC 1/1] Make gcc warning about empty if body go away.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano wrote:
Pierre Habouzit <madcoder@xxxxxxxxxx> writes:

diff --git a/builtin-diff.c b/builtin-diff.c
index f77352b..80392a8 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -204,7 +204,7 @@ static void refresh_index_quietly(void)
 		if (write_cache(fd, active_cache, active_nr) ||
 		    close(fd) ||
 		    commit_locked_index(lock_file))
-			; /*
+			(void)0; /*
 			   * silently ignore it -- we haven't mucked
 			   * with the real index.
 			   */

Wouldn't this be much easier to read, by the way?

The point is that if we touched the active_cache, we try to
write it out and make it the index file for later users to use
by calling "commit", but we do not really care about the failure
from this sequence because it is done purely as an optimization.

The original code called three functions primarily for their
side effects, which is admittedly a bad style.

 builtin-diff.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index f77352b..906c924 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -200,15 +200,9 @@ static void refresh_index_quietly(void)
 	discard_cache();
 	read_cache();
 	refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
-	if (active_cache_changed) {
-		if (write_cache(fd, active_cache, active_nr) ||
-		    close(fd) ||
-		    commit_locked_index(lock_file))
-			; /*
-			   * silently ignore it -- we haven't mucked
-			   * with the real index.
-			   */
-	}
+	if (active_cache_changed &&
+	    !write_cache(fd, active_cache, active_nr) && !close(fd))
+		commit_locked_index(lock_file);
 	rollback_lock_file(lock_file);
 }

Ack, obviously, as it no longer requires a comment to explain it, although
I'd prefer an empty line after commit_locked_index(lock_file); so as to not
confuse the rollback_lock_file() statement as being part of the conditional
path.

First I thought the rollback_lock_file() was the *only* statement to the
condition, and everyone who uses 4 for tabsize) will have double trouble
since commit_locked_index(lock_file) aligns with the second line of the
condition.

--
Andreas Ericsson                   andreas.ericsson@xxxxxx
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux