Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 09 Oct 2011 15:53:44 +0400
From:      "Alexander V. Chernikov" <melifaro@FreeBSD.org>
To:        Gleb Smirnoff <glebius@FreeBSD.org>
Cc:        svn-src-head@FreeBSD.org, "Andrey V. Elsukov" <ae@FreeBSD.org>, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org
Subject:   Re: svn commit: r225586 - in head/sys: modules/netgraph/ipfw netgraph
Message-ID:  <4E918B48.5080408@FreeBSD.org>
In-Reply-To: <20111009063420.GZ94905@FreeBSD.org>
References:  <201109151228.p8FCSHVY073618@svn.freebsd.org> <20111009063420.GZ94905@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
--------------enig5083CEE90FB8C4014FC03A03
Content-Type: multipart/mixed; boundary="------------080608030806000102020101"

This is a multi-part message in MIME format.
--------------080608030806000102020101
Content-Type: text/plain; charset=KOI8-R
Content-Transfer-Encoding: quoted-printable

Gleb Smirnoff wrote:
>   Alexander, Andrey,
>=20
>   see a couple of comments below please.
=2E..
> A> +	if (m->m_len < sizeof(struct ip) &&
> A> +	    (m =3D m_pullup(m, sizeof(struct ip))) =3D=3D NULL)
> A> +		return (EINVAL);
>=20
> In most cases we return ENOBUFS in case if m_pullup() failure. Lesser a=
mount
> of code uses ENOMEM and EINVAL. I personally hate EINVAL, since it is u=
sually
> used as one-for-all error, and thus is meaningless for user.
Understood. So can we use more descriptive ENOENT in code below?

        tag =3D m_tag_locate(m, MTAG_IPFW_RULE, 0, NULL);
        if (tag =3D=3D NULL) {
                NG_FREE_M(m);
                return (EINVAL);        /* XXX: find smth better */
        };


>=20
> A> +	ip =3D mtod(m, struct ip *);
> A> +
=2E...
> A> +		default:
> A>  			return (EINVAL);
>=20
> Shouldn't you free the mbuf before returning?
Ups.

Please see an attached patch






--------------080608030806000102020101
Content-Type: text/plain;
 name="ng_ipfw.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: inline;
 filename="ng_ipfw.diff"

Index: sys/netgraph/ng_ipfw.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- sys/netgraph/ng_ipfw.c	(revision 226165)
+++ sys/netgraph/ng_ipfw.c	(working copy)
@@ -242,7 +242,7 @@ ng_ipfw_rcvdata(hook_p hook, item_p item)
=20
 	if (m->m_len < sizeof(struct ip) &&
 	    (m =3D m_pullup(m, sizeof(struct ip))) =3D=3D NULL)
-		return (EINVAL);
+		return (ENOBUFS);
=20
 	ip =3D mtod(m, struct ip *);
=20
@@ -252,18 +252,14 @@ ng_ipfw_rcvdata(hook_p hook, item_p item)
 #ifdef INET
 		case IPVERSION:
 			ip_input(m);
-			break;
+			return (0);
 #endif
 #ifdef INET6
 		case IPV6_VERSION >> 4:
 			ip6_input(m);
-			break;
+			return (0);
 #endif
-		default:
-			NG_FREE_M(m);
-			return (EINVAL);
 		}
-		return (0);
 	} else {
 		switch (ip->ip_v) {
 #ifdef INET
@@ -277,10 +273,12 @@ ng_ipfw_rcvdata(hook_p hook, item_p item)
 			return (ip6_output(m, NULL, NULL, 0, NULL,
 			    NULL, NULL));
 #endif
-		default:
-			return (EINVAL);
 		}
 	}
+
+	/* unknown IP protocol version */
+	NG_FREE_M(m);
+	return (EPROTONOSUPPORT);
 }
=20
 static int

--------------080608030806000102020101--

--------------enig5083CEE90FB8C4014FC03A03
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (FreeBSD)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk6Ri0sACgkQwcJ4iSZ1q2nWGgCfX/EJZpmQCskKQxphoCpQ3TMn
2U4An1Tmbj4lAvEhY77Vp1LULJpRKfOo
=8LMM
-----END PGP SIGNATURE-----

--------------enig5083CEE90FB8C4014FC03A03--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E918B48.5080408>