Date: Thu, 20 Sep 2007 19:39:27 -0500 From: Brooks Davis <brooks@FreeBSD.org> To: Andrew Thompson <thompsa@FreeBSD.org> Cc: FreeBSD-net <freebsd-net@FreeBSD.org> Subject: Re: ifconfig patch Message-ID: <20070921003927.GB77167@lor.one-eyed-alien.net> In-Reply-To: <20070920235427.GA46172@heff.fud.org.nz> References: <20070920235427.GA46172@heff.fud.org.nz>
next in thread | previous in thread | raw e-mail | index | archive | help
--LpQ9ahxlCli8rRTG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Sep 21, 2007 at 11:54:27AM +1200, Andrew Thompson wrote:
> Hi,
>=20
>=20
> I have been digging into why the edsc module wasnt being loaded by
> ifconfig and now have a patch.
>=20
> A few printfs showed the problem.
>=20
> # ifconfig edsc0 create
> ifmaybeload(edsc0)
> trying to find if_edsc or edsc0
> found @ ed
>=20
> Its comparing using the string length of the module name so any partial
> matches are going through. I have changed it so it strips the number
> from the interface name and uses the full string to match.
>=20
> I want to ask re@ soon so any feedback would be great.
Conceptually the patch seems right. A couple comments below (I saw the str=
lcpy
change).
-- Brooks
> Index: ifconfig.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
> RCS file: /home/ncvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.133
> diff -u -p -r1.133 ifconfig.c
> --- ifconfig.c 13 Jun 2007 18:07:59 -0000 1.133
> +++ ifconfig.c 20 Sep 2007 23:47:28 -0000
> @@ -897,7 +897,7 @@ ifmaybeload(const char *name)
> {
> struct module_stat mstat;
> int fileid, modid;
> - char ifkind[35], *dp;
> + char ifkind[35], ifname[32], *dp;
> const char *cp;
Any reason ifname[32] shouldn't be ifname[IF_NAMESIZE]?
> /* loading suppressed by the user */
> @@ -911,6 +911,12 @@ ifmaybeload(const char *name)
> *dp =3D *cp;
> *dp =3D 0;
> =20
> + /* trim the interface number off the end */
> + strcpy(ifname, name);
> + for (dp =3D ifname; *dp !=3D 0; dp++)
> + if (isdigit(*dp))
> + *dp =3D '\0';
> +
Should the if statement terminate the loop?
> /* scan files in kernel */
> mstat.version =3D sizeof(struct module_stat);
> for (fileid =3D kldnext(0); fileid > 0; fileid =3D kldnext(fileid)) {
> @@ -926,8 +932,8 @@ ifmaybeload(const char *name)
> cp =3D mstat.name;
> }
> /* already loaded? */
> - if (strncmp(name, cp, strlen(cp)) =3D=3D 0 ||
> - strncmp(ifkind, cp, strlen(cp)) =3D=3D 0)
> + if (strncmp(ifname, cp, strlen(ifname)) =3D=3D 0 ||
> + strncmp(ifkind, cp, strlen(ifkind)) =3D=3D 0)
> return;
> }
> }
--LpQ9ahxlCli8rRTG
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (FreeBSD)
iD8DBQFG8xK/XY6L6fI4GtQRAq5HAJ9jO30nFT8xFw6rAtot9i8iJtYTDgCfUm1g
G9nGqdh1sK0JMBO+cdF7fIU=
=QCTF
-----END PGP SIGNATURE-----
--LpQ9ahxlCli8rRTG--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070921003927.GB77167>