Re: Module problem on amd64

[Andrew Doran <ad%netbsd.org@localhost>]

On Fri, Nov 28, 2008 at 04:54:46PM -0800, Paul Goyette wrote:

> Well, even if noone else thinks it is useful, I went and did the deed!

Cool, thanks! I think there is MD code that plays with 'mountroot'. See:

http://nxr.homeunix.org/source/search?q=&defs=&refs=mountroot&path=&hist=

> TEST1 and TEST2 boot successfully to single user, and I'm able to do a 
> 'ls' command from the single-user shell prompt.  However, both of these
> panic due to the "end <= VM_MAX_KERNEL_ADDRESS" thing mentioned in an 
> earlier thread.

I will fix that soon.

> +             /*
> +              * If user specified a file system type, use only
> +              * that type
> +              */
> +             if (strcmp(rootfstype, "?") && strcmp(rootfstype, v->vfs_name))
> +                     continue;

KNF says 'strcmp(a, b) == 0'

> +     static char my_vfs_name[NAME_MAX + 1];

Should be MNAMELEN from mount.h, I think.

> +                             /*
> +                              * Copy the vfs_name since *vops might be
> +                              * deallocated?
> +                              */
> +                             strncpy(my_vfs_name, vops->vfs_name,
> +                                     sizeof(my_vfs_name));
> +                             rootfstype = (const char *)my_vfs_name;
>                               vfs_delref(vops);
>                               break;

Can you make 'rootfstype' in ioconf.c (or wherever it is emitted) into an
array sized MNAMELEN and not a pointer? That way, we can get rid of
'mountroot' completely and always key on the name. So the code in
vfs_mountroot() would look like:

        if (strcmp(rootfstype, "?") != 0 &&
            (v = vfs_getopsbyname(rootfstype)) != NULL) {
                if (v->vfs_mountroot != NULL) {
                        error = (*(v->vfs_mountroot))();
                        vfs_delref(v);
                        goto done;
                } else {
                        vfs_delref(v);
                }
        }

I have just noticed a bug in the existing code. In setroot():

http://nxr.homeunix.org/source/xref/sys/kern/kern_subr.c#928

If vops->vfs_mountroot == NULL, it should do vfs_delref(vops) before
continuing on.

Thanks,
Andrew