[Toybox] toolbox ioctl.c

[enh]

On Sat, Oct 31, 2015 at 3:33 PM, Rob Landley <rob at landley.net> 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.

 1446496764.0