On Fri, Jan 14, 2011 at 06:29:22PM +0100, Jan Engelhardt wrote:
>
> On Friday 2011-01-14 17:59, Thomas Graf wrote:
> >
> >This patch adds a new netfilter target which creates audit records
> >for packets traversing a certain chain.
> >+#ifndef _XT_AUDIT_TARGET_H
> >+#define _XT_AUDIT_TARGET_H
> >+
> >+#include <linux/types.h>
> >+
> >+enum {
> >+ XT_AUDIT_TYPE_ACCEPT = 0,
> >+ XT_AUDIT_TYPE_DROP,
> >+ XT_AUDIT_TYPE_REJECT,
> >+ __XT_AUDIT_TYPE_MAX,
> >+};
> >+
> >+#define XT_AUDIT_TYPE_MAX (__XT_AUDIT_TYPE_MAX - 1)
>
> Hm, why not just add to the enum:
The above is used in various places around the kernel and there
is nothing wrong with it.
> >+static int audit_tg_check(const struct xt_tgchk_param *par)
> >+{
> >+ const struct xt_AUDIT_info *info = par->targinfo;
> >+
> >+ if (info->type > XT_AUDIT_TYPE_MAX) {
> >+ pr_info("Audit type out of range (valid range: 0..%u)\n",
> >+ XT_AUDIT_TYPE_MAX);
> >+ return -ERANGE;
> >+ }
> >+
> >+ return 0;
> >+}
>
> Math nitpick: EDOM, not ERANGE.
ERANGE is the common error code to use in this situation.
> Do we need __XT_AUDIT_TYPE_MAX? It is unused; that is to say,
> would not this suffice:
>
> enum {
> ...,
> XT_AUDIT_TYPE_WHATEVER,
> - __XT_AUDIT_TYPE_MAX,
> - XT_AUDIT_TYPE_MAX = __XT_AUDIT_TYPE_MAX - 1,
> + XT_AUDIT_TYPE_MAX = XT_AUDIT_TYPE_WHATEVER,
> };
This requires to modify XT_AUDIT_TYPE_MAX whenever the list is
extended which is a pain to maintain. Let's leave it how it is,
it's a well known coding practice and known to work just fine.
--
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
