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;
> }
> }
pgpbbQkrpdO2D.pgp
Description: PGP signature
