On Wed, Aug 3, 2016 at 2:37 PM, Lars Schneider <larsxschneider@xxxxxxxxx> wrote: >> >> I think this was already pointed out in the previous review by Peff, >> but a variable "ret" that says "0 is bad" somehow makes it hard to >> follow the code. Perhaps rename it to "int error", flip the meaning, >> and if the caller wants this function to return non-zero on success >> flip the polarity in the return statement itself, i.e. "return !errors", >> may make it easier to follow? > > This follows the existing filter function. Please see Peff's later > reply here: Which I did before mentioning "pointed out in his review". > That's why I kept it the way it is. If you prefer the "!errors" approach > then I will change that. I am not suggesting to change the RETURN VALUE from this function. That is why I mentioned "return !errors" to flip the polarity at the end. Inside the function, "ret" variable _forces_ the readers to think "this function unlike the others signal an error with 0" constantly while reading it, and one possible approach to reduce the mental burden is to replace "ret" variable with "errors" variable, which is clear to anybody that it would be non-zero when we saw error(s). Oh, I am not suggesting to _count_ the number of errors by mentioning a possible variable name "errors"; the only reason why I mentioned that name is because "error" is already taken, and "seen_error" is a bit too long. -- 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