Re: [PATCH 1/2] merge-recursive: Workaround unused variable warning
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 18, 2010, 18:55 UTC
- Message-ID
- <7veidvagz8.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20100818001032.GA7694@burratino>
Jonathan Nieder <jrnieder@gmail.com> writes:
Show 16 quoted lines
> 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);