On Wed, Jan 21, 2015 at 09:46:46PM +0100, Gabriel Laskar wrote: > Some ioctls from evdev, hiddev, mixer, uinput, spi and joystick are > parametered by a size or a number that is variable. These changes add > the display of these ones, and their parameters. > > * defs.h: Declare new function ioctl_decode_number.
This line should be * defs.h (ioctl_decode_number): New prototype. > * io.c (sys_ioctl): Add call to ioctl_decode_number. This line should be * io.c (sys_ioctl): Use ioctl_decode_number. or even * io.c (sys_ioctl): Use it. if the next line is placed before it. > * ioctl.c (ioctl_decode_number): Implement this new function. This line should be * ioctl.c (ioctl_decode_number): New function. > * xlat/evdev_abs.in: new file. > * xlat/evdev_ev.in: new file. These sentences should start with a capital letter. For more information about ChangeLog style, please refer to https://www.gnu.org/prep/standards/html_node/Change-Logs.html I suppose we will stop this practice of writing ChangeLog-style entries in commit messages some day, but this won't happen today. :) [...] > --- a/ioctl.c > +++ b/ioctl.c > @@ -32,6 +32,11 @@ > #include <asm/ioctl.h> > #include "xlat/ioctl_dirs.h" > > +#include <linux/input.h> Probably OK to include this header file unconditionally. If any issues arises, they could be dealt with. > + > +#include "xlat/evdev_abs.h" > +#include "xlat/evdev_ev.h" > + > static int > compare(const void *a, const void *b) > { > @@ -77,6 +82,142 @@ ioctl_print_code(const unsigned int code) > _IOC_TYPE(code), _IOC_NR(code), _IOC_SIZE(code)); > } > > +static int > +evdev_decode_number(unsigned int arg) > +{ > + unsigned int nr = _IOC_NR(arg); > + > + if (_IOC_DIR(arg) == _IOC_WRITE) { > + if (nr >= 0xc0 && nr <= 0xc0 + 0x3f) { > + tprints("EVIOCSABS("); > + printxval(evdev_abs, nr - 0xc0, "invalid absolute axe"); There might be other reasons for this value not to be found in evdev_abs, for example, the version of linux/input.h used to built strace had no definition for this value. That's why we are not so resolute in cases like this. Let's use a more neutral string like "ABS_???". > + tprints(")"); > + return 1; > + } > + } > + > + if (_IOC_DIR(arg) != _IOC_READ) > + return 0; > + > + if (nr >= 0x20 && nr <= 0x20 + 0x1f) { > + tprints("EVIOCGBIT("); > + printxval(evdev_ev, nr - 0x20, "invalid event type"); Same here, let's use "EV_???" instead. [...] > +int > +ioctl_decode_command_number(unsigned int arg) > +{ > + switch (_IOC_TYPE(arg)) { > + case 'E': > + return evdev_decode_number(arg); > + case 'H': > + return hiddev_decode_number(arg); > + case 'M': > + if (_IOC_DIR(arg) == _IOC_WRITE) { > + tprintf("MIXER_WRITE(%u)", _IOC_NR(arg)); > + } else if (_IOC_DIR(arg) == _IOC_READ) { > + tprintf("MIXER_READ(%u)", _IOC_NR(arg)); > + } > + return 1; What if _IOC_DIR(arg) is neither _IOC_WRITE nor _IOC_READ? Please do "return 1;" consistently. -- ldv
pgp1R1J1og3PQ.pgp
Description: PGP signature
------------------------------------------------------------------------------ New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet
_______________________________________________ Strace-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/strace-devel
