Re: [PATCH v2 2/5] ipset: add "inner" flag implementation

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

 




Jozsef Kadlecsik wrote:
As you've probably guessed, I kept the old functions and their parameters in
order to preserve the existing interface API, since any changes I make to
these will break existing code using them. If there is no objection and there
is no such requirement, I'll get rid of them in the next release of the
patches - just let me know.

Yes, there's no such requirement and remove the backward compatibility part.
OK, I'll remove it then.

The return value of ip[6]_hdr was never checked in ipset code yet, because it doesn't return NULL at the point where these code parts are called. It's an unnecessary checking.
Fair enough - I'll remove that check too.

I disagree. By having "return false" (or "return 0", "return -1" and so on)
instead of "goto err" it is not immediately apparent to someone who
studies/reviews/uses the code that this is an error condition/path. I have
been in that situation many times when I have to go back and look at a
particular function call to see what "return false" or "return 0" actually
means.

By including "goto err" instead of multiple "return false" statement, that
makes it instantly obvious what the purpose of that statement is without
having to look elsewhere.

I see the point, to self-document the code, with the price of more lines. It's a little bit overdoing in my opinion: pretty apparent which is the error path and which is not. But this is highly a personal taste, I won't press it.
In other words, you'll be happy for me to leave things as they are - with the "goto"?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux