Re: CVS commit: basesrc/lib/libutil

["R.o.s.s H.a.r.v.e.y" <ross%ghs.com@localhost>]

> From: Luke Mewburn <lukem%wasabisystems.com@localhost>
>
> On Sat, Dec 08, 2001 at 10:58:45AM -0800, Ross Harvey wrote:
> > I suppose this isn't a very important issue (netbsd-specific internal
> > library interface and all that) but could you expand on the reason
> > for this additional check?
> > 
> > I'm sure I'm just missing a perfectly good reason for doing it,
> > but at first glance it seems like it just takes some calls that
> > were about to succeed and errors them out.
>
> If the caller of opendisk(3) specifically wanted a BLK (iscooked != 0)
> or CHR (iscooked = 0), then I saw no problem in enforcing that requirement
> in opendisk(3) to ensure that the caller obtains a file descriptor of the
> appropriate type. In hindsight, I should have had those checks in the code
> when I first implemented opendisk(3) over four years ago.

But the caller may be able to operate on _either_ class of device.
This is the case with vnconfig(8) and disklabel(8).  Plus, at times
we have wanted to run some utilities on plain files without using
vnd(4), but now they must bypass opendisk(3).

> This changes the user visible behaviour from:
>       % echo "foo" > wd0
>       % disklabel wd0
>       disklabel: can't read master boot record: Undefined error: 0
> to:
>       % echo "foo" > wd0
>       % disklabel wd0
>       disklabel: wd0: Operation not supported by device
> (although wd0 could be in `...' to make it more obvious)

I certainly see the need for a better error message from disklabel(8),
but I'm not impressed by the improvement; a far better message
would have resulted from directly improving disklabel(8).

Changing the opendisk(3) API from `iscooked specifies optional
rewriting' to the current meaning seems like a severe and indirect
way of fixing disklabel(8).

It broke four of the seven Makefile's (some, in several places) under
distrib/alpha, which is no big deal, but there may be scripts in
the field that use vnconfig(8), disklabel(8), scsictl(8), or atactl(8),
and I'm not at all sure that sysinst is still working.

> I also improved the way that the rules for testing the variants of the
> argument (e.g, 'wd0' -> 'wd0d', `/dev/rwd0', `/dev/rwd0d').

What?

>
> > E.g. (and ok, sure, one should really let opendisk(3) find the raw
> > partition pathname) the following _used_ to work and now fails:
> > 
> >     vnconfig /dev/vnd2c ...
>
> Well, vnconfig was asking for a character device from opendisk(3) and
> you gave it the block one...

No, vnconfig was not.

vnconfig(8) and disklabel(8) _had_ to specify something (no wildcard
in the API) but they didn't care which they got.  They formerly
worked just fine with either block or char. And what if a plain
file would have worked?

Using disklabel is hard enough on some platforms, now we've made
half of the possible legal uses error out, just to indirectly get
a only slightly better perror(3) on already illegal cases.

        r.o.s.s