CVS commit: src/sbin/raidctl

["Robert Elz" <kre%netbsd.org@localhost>]

Module Name:    src
Committed By:   kre
Date:           Tue Jun 14 08:06:13 UTC 2022

Modified Files:
        src/sbin/raidctl: rf_configure.c

Log Message:
Fix some config file parsing.

First, and what got me started on this set of cleanups, the queue
length in the "queue" section (START queue) is limited to what will
fit in a char without losing accuracy (I tried setting it to 200,
rather than the more common (universal?) 100 and found that the
value configured into the array was -56 instead.

Why the value needs to be passed through a char variable I have no
idea (it is an int in the filesystem raidframe headers) - but that's
the way it is done, and changing it would be an ABI change I believe
(and so need versioning to alter) and that isn't worth it for this
(or not now, IMO).

Instead check that the value in the char is the same value as was
read from the config file, and complain if not.   Those of you with
unsigned chars will be able to have queue lengths up to 255, the
rest of us are limited to 127.

While looking at that, I noticed some code that obviously fails to
understand that scanf("%s") will never return a string containing
spaces, and proceeded to attempt to remove trailing spaces from the
result ... amusingly, after having used the result for its intended
purpose (non existent trailing spaces unremoved), after which that
buffer was never used again.   That code is now gone (but for now,
just #if 0'd rather than actually deleted - it should be cleaned up
sometime).

Then I saw some other issues with how the config was parsed - a
simple (unbounded) scanf("%s") into a buffer, which hypothetically
might not be large enough (not a security issue really, raidctl has
no special privs, and it isn't likely that root could easily be
tricked into running it on a bogus config file - or not without
looking first anyway, and a huge long string would rather stand
out).   Bound the string length to something reasonable, and
assert() that the buffer is big enough to contain it.

Lastly, in the event of one particular detected error in the
config file, the code would write a warning, but then just go
ahead and use the bad data (or nothing perhaps) anyway - a
failure of logic flow (unlikely to have ever happened, everyone
seems to simply copy the sample config from the man page, and
make minor adjustments as needed).

If any of these changes make any difference to anyone (except
me with my attempt to make longer queues - for no particularly
well thought out reason), I'd be very surprised.


To generate a diff of this commit:
cvs rdiff -u -r1.35 -r1.36 src/sbin/raidctl/rf_configure.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.