install/55379: sysinst creates suspicious "/dev/ /cdrom cd9660 ro,noauto" in fstab

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

>Number:         55379
>Category:       install
>Synopsis:       sysinst creates suspicious "/dev/ /cdrom cd9660 ro,noauto" in fstab
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    install-manager
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Fri Jun 12 18:15:00 +0000 2020
>Originator:     Izumi Tsutsui
>Release:        NetBSD 9.0
>Organization:
>Environment:
System: NetBSD 9.0 (RAMDISK) #0: Fri Feb 14 00:06:28 UTC 2020
mkrepro%mkrepro.NetBSD.org@localhost:/usr/src/sys/arch/cobalt/compile/RAMDISK
Architecture: maybe all
Machine: all
>Description:
After installation on Gxemul cobalt emulation without CD-ROM,
sysinst creates the following /targetroot/etc/fstab:

---
# mount /dev/wd0a /targetroot
# cat /targetroot/etc/fstab
# NetBSD /etc/fstab
# See /usr/share/examples/fstab/ for more examples.
/dev/wd0a               /       ffs     rw               1 1
# /dev/wd0e             /stand  ???     rw               0 0
/dev/wd0b               none    swap    sw,dp            0 0
kernfs          /kern   kernfs  rw
ptyfs           /dev/pts        ptyfs   rw
procfs          /proc   procfs  rw
/dev/           /cdrom  cd9660  ro,noauto
tmpfs           /var/shm        tmpfs   rw,-m1777,-sram%25
# 
---

I wonder if "/dev/" for /cdrom is intentional.

>How-To-Repeat:
Proceed installation by sysinst on systems without CD-ROM drive.

>Fix:
In src/usr.sbin/sysinst/disks.c, it looks to try to detect
a proper device name for a CD-ROM drive:
---
/*
 * Helper function for get_default_cdrom, gets passed a device
 * name and a void pointer to default_cdrom_data.
 */
static bool
get_default_cdrom_helper(void *state, const char *dev)
{
	struct default_cdrom_data *data = state;

	if (!is_cdrom_device(dev, false))
		return true;

	strlcpy(data->device, dev, data->max_len);
	strlcat(data->device, "a", data->max_len); /* default to partition a */
	data->found = true;

	return false;	/* one is enough, stop iteration */
}

/*
 * Set the argument to the name of the first CD devices actually
 * available, leave it unmodified otherwise.
 * Return true if a device has been found.
 */
bool
get_default_cdrom(char *cd, size_t max_len)
{
	struct default_cdrom_data state;

	state.device = cd;
	state.max_len = max_len;
	state.found = false;

	if (enumerate_disks(&state, get_default_cdrom_helper))
		return state.found;

	return false;
}

 :

done_with_disks:
	if (cdrom_dev[0] == 0)
		get_default_cdrom(cdrom_dev, sizeof(cdrom_dev));

	/* Add /kern, /proc and /dev/pts to fstab and make mountpoint. */
	scripting_fprintf(f, "kernfs\t\t/kern\tkernfs\trw\n");
	scripting_fprintf(f, "ptyfs\t\t/dev/pts\tptyfs\trw\n");
	scripting_fprintf(f, "procfs\t\t/proc\tprocfs\trw\n");
	scripting_fprintf(f, "/dev/%s\t\t/cdrom\tcd9660\tro,noauto\n",
	    cdrom_dev);

---

But there is no check a return value of the get_default_cdrom().

I don't know the name "/dev/" is intentional, but I think
it's much better to choose either of the following:
(1) Omit the  "/dev/ /cdrom noauto" line if no CD-ROM is detected
(2) Use "/dev/cd0a" by default

Actually /dev/cd0a is appropriate on most systems.
(exceptions are only rare mcd(4) and gdrom(4)?)

Note there are functional duplication of "detect CD-ROM media"
in src/usr.sbin/sysinst/utils.c.

---
Izumi Tsutsui