Torsten Bögershausen <tboegi@xxxxxx> writes: > On 07/13/2014 10:28 PM, David Turner wrote: >> From: David Turner <dturner@xxxxxxxxxxxxxxxx> > [] >> diff --git a/cache-tree.c b/cache-tree.c >> index 7fa524a..f951d7d 100644 >> --- a/cache-tree.c >> +++ b/cache-tree.c >> @@ -239,9 +239,12 @@ static int update_one(struct cache_tree *it, >> struct strbuf buffer; >> int missing_ok = flags & WRITE_TREE_MISSING_OK; >> int dryrun = flags & WRITE_TREE_DRY_RUN; >> + int repair = flags & WRITE_TREE_REPAIR; >> int to_invalidate = 0; >> int i; >> + assert(!(dryrun && repair)); > I think something in the spirit of > die("dryrun and repaiir can not be used together"\n) > Would be nicer to the user as well as being more reliable (as assert > may be a no-op in some systems) While it is a good suggestion *not* to attempt validating the end-user input with assert() for the reason you state, I think for this particular case, these flags only come from the code and assert() to catch programming errors would be sufficient. Besides, as discussed elsewhere, WRITE_TREE_DRY_RUN should not be used, and the support for it should be dropped. It is broken in that the code path that leads to update_one() may correctly compute tree object names and stuff them in the cache-tree, higher layer code would then complain on such a cache-tree that records tree objects that do not exist. -- 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