Re: fdformat(8) doesn't work

On Sun, 25 Apr 2004, Ruslan Ermilov wrote:

> The subject says it all, really.  This is with -CURRENT,
> and it didn't work for quite some time already.

It was broken in rev.1.266 by removing the wrong half of an if-else
clause.  I sent a patch to the maintainer a couple of weeks ago but
have received no reply.  Meanwhile, fdc itself was broken by pci BAR
changes.

This patch has only been tested in my version of the fd.c driver which
has 16KB of other patches.  The XXX comment is about stuff that was
bogotified by rev.1.266, and possible problems with the lifetime of
fd->ft.  s/+/|/ is an unrelated cleanup.  Removal of the redundant
FD_GTYPE case is a related cleanup.

%%%
Index: fd.c
===================================================================
RCS file: /home/ncvs/src/sys/isa/fd.c,v
retrieving revision 1.269
diff -u -2 -r1.269 fd.c
--- fd.c	7 Apr 2004 20:46:08 -0000	1.269
+++ fd.c	12 Apr 2004 23:45:38 -0000
_at__at_ -2601,11 +2774,25 _at__at_

 	case FD_STYPE:                  /* set drive type */
-		if (suser(td) != 0)
-			return (EPERM);
+		/*
+		 * Allow setting drive type temporarily iff
+		 * currently unset.  Used for fdformat so any
+		 * user can set it, and then start formatting.
+		 */
+		if (fd->ft)
+			return (EINVAL); /* already set */
+		/*
+		 * XXX the following is the only use of fts[].  fts[0] only
+		 * provides a place to point fd->ft at here.  All other fts
+		 * elements are unused, but all elements are initialized.
+		 * fd->ft only lives until the next open.  I think the next
+		 * open can clobber an active fd->ft.
+		 */
 		fd->fts[0] = *(struct fd_type *)addr;
+		fd->ft = &fd->fts[0];
+		fd->flags |= FD_UA;
 		return (0);

 	case FD_GOPTS:			/* get drive options */
-		*(int *)addr = fd->options + FDOPT_AUTOSEL;
+		*(int *)addr = fd->options | FDOPT_AUTOSEL;
 		return (0);

_at__at_ -2663,8 +2850,4 _at__at_
 		break;

-	case FD_GTYPE:                  /* get drive type */
-		*(struct fd_type *)addr = *fd->ft;
-		break;
-
 	case FD_STYPE:                  /* set drive type */
 		/* this is considered harmful; only allow for superuser */
%%%

Bruce