From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 18 Aug 2010 18:55:07 GMT
Subject: Re: [PATCH 1/2] merge-recursive: Workaround unused variable warning
Message-ID: <7veidvagz8.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100818001032.GA7694@burratino>

Jonathan Nieder <jrnieder@gmail.com> writes:

> Elijah Newren wrote:
>> On Thu, Aug 12, 2010 at 8:09 PM, Elijah Newren <newren@gmail.com> wrote:
>
>>> +++ b/merge-recursive.c
>>> @@ -1214,6 +1214,7 @@ static int process_df_entry(struct merge_options *o,
>>>        /* We currently only handle D->F cases */
>>>        assert((!o_sha && a_sha && !b_sha) ||
>>>               (!o_sha && !a_sha && b_sha));
>>> +       (void)o_sha;
> [...]
>>                        would a different method of fixing warnings
>> when NDEBUG is defined be preferred?  (Maybe changing the
>> "assert(foo)" into "if (!foo) die..." instead?)
>
> Yes, that sounds like a good idea.  The user would probably benefit
> from knowing what happened.

I'd agree.  This assert() is not about protecting new callers from making
obvious programming error by passing wrong parameters, but about Elijah
not being confident enough that the changes made to process_entry() with
this series really covers all the cases so that only D/F cases are left
unprocessed.

Another possibility is to move this assert() out of process_df_entry() and
have it on the calling side.  Perhaps something like the attached.

BTW, it is not so obvious if (!o_sha && !!a_sha != !!b_sha) is equivalent
to "we are handling a D-F case".  Can you explain why?


diff --git a/merge-recursive.c b/merge-recursive.c
index b0f055e..7bab728 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1208,9 +1208,8 @@ static int process_df_entry(struct merge_options *o,
 	const char *conf;
 	struct stat st;
 
-	/* We currently only handle D->F cases */
-	assert((!o_sha && a_sha && !b_sha) ||
-	       (!o_sha && !a_sha && b_sha));
+	if (! ((!o_sha && a_sha && !b_sha) || (!o_sha && !a_sha && b_sha)))
+		return 1; /* we don't handle non D-F cases */
 
 	entry->processed = 1;
 
@@ -1321,6 +1320,12 @@ int merge_trees(struct merge_options *o,
 				&& !process_df_entry(o, path, e))
 				clean = 0;
 		}
+		for (i = 0; i < entries->nr; i++) {
+			struct stage_data *e = entries->items[i].util;
+			if (!e->processed)
+				die("Unprocessed path??? %s", 
+				    entries->items[i].string);
+		}
 
 		string_list_clear(re_merge, 0);
 		string_list_clear(re_head, 0);