On Fri, Jul 11, 2014 at 11:45:58AM +0100, Ramsay Jones wrote: > > @@ -1729,9 +1729,8 @@ static enum peel_status peel_object(const unsigned char *name, unsigned char *sh > > > > if (o->type == OBJ_NONE) { > > int type = sha1_object_info(name, NULL); > > - if (type < 0) > > + if (type < 0 || !object_as_type(o, type, 0)) > --------------------------------------------------------^^^ > > It is not possible here for object_as_type() to issue an error() > report, but I had to go look at the code to check. (It would have > been a change in behaviour, otherwise). So, it doesn't really matter > what you pass to the quiet argument, but setting it to 1 _may_ help > the next reader. (No, I'm not convinced either; the only reason I > knew it had anything to do with error messages was because I had > just read the code ...) Hmm, dunno. Right, as I mentioned in the commit message, the type-check part of object_type is a noop here. In that sense you could write this as just: object_as_type(o, type. 1); and ignore the return value. However, I'd rather err on the side of checking for and reporting the error, even if we expect it to do nothing right now. That decouples the two functions, and if object_as_type ever learns to report other errors, then we automatically do the right thing here. -Peff -- 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