On Sat, Oct 31, 2015 at 3:33 PM, Rob Landley <[email protected]> wrote: > I've been looking at ioctl.c in toolbox, and it's... confusing.
it's a bringup team thing, so that's to be expected :-) > The help doesn't mention that you have more arguments after <device> and > <ioctlnr>, but most of the logic parses them. > > I'm not sure when/how -a would get used? If you're passing in a struct > you need to specify varying sizes for varying fields, this is the same > size for each field. The default behavior is to treat each argument as > an -a 4 int (fair enough) but the parsing assumes little-endian: (Android is little-endian, and so are all commercially relevant hosts, so no one on Android cares about big-endian.) > uint64_t tmp = strtoull(argv[optind], NULL, 0); > ... > memcpy(ioctl_argp, &tmp, arg_size); > > I don't understand how -d "direct" mode works. It turns an int into a > pointer (so that's a 32-bit assumption there) and then dereferences that > pointer: > > if(direct_arg) > res = ioctl(fd, ioctl_nr, *(uint32_t*)ioctl_args); > > But it's a pointer to... what? The pointer value to dereference is > passed in on the command line. (This seems very much to be a "it breaks > you keep the pieces" mode, I.E. if you don't pass in an extra argument > it'll default to 0 so we dereference a null pointer by _default_?) it may have been useful to someone once and checked in anyway. i'd remove it. > The output is always verbose human-readable stuff, always issued with no > -v option. There's no "directly output the data we got back" mode that > doesn't hex-encode it. > > The return value is always zero, not the return code of the ioctl. > > We're specifying buffer length on the command line even though ioctl > numbers encode their input/output direction and the size of their > parameter structure in the ioctl number, from asm-generic/ioctl.h: > > /* ioctl command encoding: 32 bits total, command in lower 16 bits, > * size of the parameter structure in the lower 14 bits of the > * upper 16 bits. > > It also specifies _IOC_WRITE and _IOC_READ in those top two bits to let > you know if the ioctl accepts input or produces output into the buffer. > Although the asm-generic header comment says architectures can override > that, a grep of arch/*/include/asm doesn't find any architecture > actually doing that. So I don't think we even need to #include anything > special and can just use the constants because they're > architecture-independent. > > That means we can supply arguments only when the ioctl needs arguments, > and produce output only when the ioctl produced output. (The question is > will this break existing users who are parsing the human readable output > with giant regexes?) there's nothing in the internal tree that uses this command. i was tempted to just move it to system/extras/ with other bringup crap, but if you think it can be made generally useful, i think it's fine to break compatibility. > If you don't provide enough arguments for the buffer, what's left is > zeroed. In theory we could detect "buffer size" and "requires input" and > enforce passing enough arguments when input is required. Would that be a > good thing or a bad thing? sgtm. you could always add a -f if anyone explains why they need to be able to do obviously wrong things. > If we do detect input, I think -l is only needed in the -d case? No, the > -d parsing logic unconditionally overwrite -a and -l values specified on > the command line. (So that's "[!da][!dl]" in the optarg string, really.) > > Anyway, I've got the start of a toybox ioctl implementation, but no test > cases for it. I'm not sure how much I'm allowed to change without > breaking existing users. I googled for existing users and found very > little, but I'm not sure exactly what to search for... most of the git changes were just folks keeping this building, and the pre-git history has only two tiny changes. i think this was just someone's personal test code that got checked in. > Rob -- Elliott Hughes - http://who/enh - http://jessies.org/~enh/ Android native code/tools questions? Mail me/drop by/add me as a reviewer. _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
