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>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
On Fri, Sep 21, 2007 at 11:54:27AM +1200, Andrew Thompson wrote:
> Hi,
>
>
> I have been digging into why the edsc module wasnt being loaded by
> ifconfig and now have a patch.
>
> A few printfs showed the problem.
>
> # ifconfig edsc0 create
> ifmaybeload(edsc0)
> trying to find if_edsc or edsc0
> found @ ed
>
> 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.
>
> I want to ask re@ soon so any feedback would be great.
Conceptually the patch seems right. A couple comments below (I saw the strlcpy
change).
-- Brooks
> Index: ifconfig.c
> ===================================================================
> 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 = *cp;
> *dp = 0;
>
> + /* trim the interface number off the end */
> + strcpy(ifname, name);
> + for (dp = ifname; *dp != 0; dp++)
> + if (isdigit(*dp))
> + *dp = '\0';
> +
Should the if statement terminate the loop?
> /* scan files in kernel */
> mstat.version = sizeof(struct module_stat);
> for (fileid = kldnext(0); fileid > 0; fileid = kldnext(fileid)) {
> @@ -926,8 +932,8 @@ ifmaybeload(const char *name)
> cp = mstat.name;
> }
> /* already loaded? */
> - if (strncmp(name, cp, strlen(cp)) == 0 ||
> - strncmp(ifkind, cp, strlen(cp)) == 0)
> + if (strncmp(ifname, cp, strlen(ifname)) == 0 ||
> + strncmp(ifkind, cp, strlen(ifkind)) == 0)
> return;
> }
> }
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (FreeBSD)
iD8DBQFG8xK/XY6L6fI4GtQRAq5HAJ9jO30nFT8xFw6rAtot9i8iJtYTDgCfUm1g
G9nGqdh1sK0JMBO+cdF7fIU=
=QCTF
-----END PGP SIGNATURE-----
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070921003927.GB77167>