tech-userlevel archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
ccgconfig(8) questions
Some of you might have noticed from source-changes@ or netbsd-bugs@
that I have been doing some work on ccgconfig. That was in response
to a PR of mine, which I never really thought anyone would bother
doing anything about (it was "low" "non-critical", and not very
important) - but someone did make the effort to send a patch for
it, so I thought I should handle it. That's done now.
But while fixing the code, I came across two additional things I
think should be fixed.
The first isn't a problem as such, just essentially dead code doing
nothing much. Long ago, ccdconfig (like many other similar utilities)
was setgid kmem (kmem I think) to allow it to read /dev/kmem to
extract information about the existing configured ccds. It no longer
works that way, and is no longer installed setgid. That's all good.
But it still has code in it to handle being installed setgid, pretending
to switch around which gid is in use, for various operations, which is
the kind of thing needed for setXid processes. That's all dead code
now right? No-one is ever going to want ccdconfig to be installed
setgid again, so that code could just be deleted, I believe.
I didn't do that yet however.
And second, this amazing piece of code (amazing to me anyway) exists.
This is in part of the replacement for being setgid code, getting info
via sysctl() instead of reading /dev/kmem.
size_t nunits = 0;
sys = "kern.ccd.units";
if (sysctlbyname(sys, NULL, &nunits, NULL, 0) == -1) {
switch (errno) {
case ENOENT:
warnx("no ccd driver in the kernel");
return 1;
case ENOMEM:
break; /* XXX ??? Huh, why? */
default:
err(EXIT_FAILURE,
"error getting %s (length)", sys);
/* NOTREACHED */
}
}
if (nunits == 0) {
warnx("no concatenated disks configured");
return 1;
}
What's going on with that ENOMEM handling? The XXX comment I added.
As written, that's just treating ENOMEM as if it were no error at all. But
I assume that if it happens, nunits isn't being set by the sysctl system call,
leaving it as 0, and so falling into the "no .. disks configured" warning
(error effectively, the return 1 turns into exit(1)) - but that would be a
completely bogus message for this scenario. The only way I can imagine
an ENOMEM happening there would be if there are ccds configured.
Am I misinterpreting what is happening there, or is that ENOMEM handling
just broken? Aside for a minor change to the text of the message in
the err() in the default case, and moving the assignment to "sys" out
of the sysctlbyname() arg list (avoids line wrapping) all I changed in
that piece of code is the XXX comment addition.
Insight welcome! (I can't imagine how I can provoke an ENOMEM to test it).
This is all in src/sbin/ccdconfig/ccdconfig.c if anyone wants to look
(just search for that XXX, there are just 2 of those in the code, this
new one, and an old one commenting on the efficacy of the vax version of gcc).
You won't miss the right one!
Oh - I understand what that code is doing in general, no need to explain it.
It is just that ENOMEM handling that I don't follow the reasoning for.
The rest is fine - though I'm not entirely sure why it does warn();return 1
in some cases, and err() in others - the return value is handed directly
to exit(), so those warn() calls could be err() equivalents instead. That's
not important however.
kre
Home |
Main Index |
Thread Index |
Old Index