[Toybox] [CLEANUP] openvt/deallocvt

Rob Landley rob at landley.net
Tue Jun 24 04:48:02 PDT 2014


vivek: could you answer the KDGKBTYPE vs VT_GETSTATE question at the end?

commit 1358: http://landley.net/hg/toybox/rev/1358

Step 1: move deallocvt.c into openvt.c so we don't have one command
laterally depending on code in another. This also lets us collate
duplicate headers.

deallocvt: do the standard toys.optargs[0] -> *toys.optargs tweaks,
tweak help text, collate variable declarations...

Hmmm, if I _don't_ make deallocvt depend on openvt (which I don't think
it needs to now?) the order in menuconfig is wrong. Everything else is
alphabetized, but this still has deallocvt right after openvt in
menuconfig. Todo item...

Eliminate redundant typecasts: ptrdiff_t immediately retypecast to void
* is a NOP, except what the first typecast does is promote to a long (to
avoid an int->pointer typecast warning on 64 bit). So the vt_num
variable should probably be a long in the first place.

openvt: rename find_console_fd() to open_console(), use ARRAY_LEN() for
the loop.

query: does the O_RDONLY/O_WRONLY open pair actually accomplish
anything? I thought root ignored permissions when opening files (you can
open a chmod 000 file as root), and these commands are TOYFLAG_NEEDROOT.
Plus devtmpfs and similar should have "rw" as the permissons on these
devices, if they don't something's wrong?

Replacing that with a single O_RDWR open, I'm happy to put it back if
you explain why it's needed...

openvt_main: fd and vt_fd are both assigned to before reading, so
initializing them to -1 is unnecessary.

query: the if (!FLAG_c) bit is doing a very similar "try fd 0-3, open
/dev/console" thing to open_console(), the difference is that's calling
ioctl(KDGKBTYPE) and this is calling ioctl(VT_GETSTATE). If the purpose
of KDGKBTYPE is to confirm this is a tty: will VT_GETSTATE do that?

I'm wondering because I'd like to collapse the two codepaths together,
but I don't understand what the code is doing well enough.
(Documentation on ioctls is hard to come by, man ioctl_list doesn't even
show KDGKBTYPE, let alone explain what VT_GETSTATE actually _does_.
Would calling VT_GETSTATE sufficiently identify "we have a tty"
especially since we're trying to call it on /dev/console and such?)

Rob


More information about the Toybox mailing list