Re: [PATCH 1/2] merge-recursive: Workaround unused variable warning

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

 



Jonathan Nieder <jrnieder@xxxxxxxxx> writes:

> Elijah Newren wrote:
>> On Thu, Aug 12, 2010 at 8:09 PM, Elijah Newren <newren@xxxxxxxxx> 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);
--
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]