[PATCH 3/3] hdio: improve HDIO support

[Srikavin Ramkumar]

On Mon Aug 30, 2021 at 7:18 AM EDT, Dmitry V. Levin wrote:
> On Sat, Aug 21, 2021 at 09:52:00AM -0400, Srikavin Ramkumar wrote:
> > Add decoders for HDIO_GET_UNMASKINTR, HDIO_SET_UNMASKINTR,
> > HDIO_GET_MULTCOUNT, HDIO_SET_MULTCOUNT, HDIO_OBSOLETE_IDENTITY,
> > HDIO_GET_IDENTITY, HDIO_GET_KEEPSETTINGS, HDIO_SET_KEEPSETTINGS,
> > HDIO_GET_32BIT, HDIO_GET_NOWERR, HDIO_GET_DMA, HDIO_GET_NICE,
> > HDIO_SET_NICE, HDIO_GET_WCACHE, HDIO_GET_ACOUSTIC, HDIO_GET_ADDRESS,
> > HDIO_GET_BUSSTATE, HDIO_SET_BUSSTATE, and HDIO_DRIVE_RESET.
> > 
> > * maint/gendefs/hdio.def: New file.
> > * src/Makefile.am (libstrace_a_SOURCES): Add gen/generated.h and gen/gen_hdio.c.
> > * src/gen/gen_hdio.c: Add generated file.
> > * src/gen/generated.h: New file.
> > * src/hdio.c: Add call to generated file for unimplemented commands.
> > * src/xlat/hdio_busstates.in: New file.
> > * src/xlat/hdio_ide_nice.in: Likewise.
> > * tests/ioctl_hdio.c: Add tests for newly implemented commands.
>
> I think this change worths a NEWS entry.

Sure, I'll add an entry to NEWS.

> [...]
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index b7e74a76e..3189abf44 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -132,6 +132,8 @@ libstrace_a_SOURCES =	\
> >  	fstatfs64.c \
> >  	futex.c		\
> >  	gcc_compat.h	\
> > +    gen/gen_hdio.c  \
> > +    gen/generated.h \
> >  	get_personality.c \
> >  	get_personality.h \
> >  	get_robust_list.c \
>
> Something went wrong with the indentation here.
>

Looks like I accidentally used spaces instead of a tab here.

> > diff --git a/src/gen/gen_hdio.c b/src/gen/gen_hdio.c
> > new file mode 100644
> > index 000000000..b558a4ebd
> > --- /dev/null
> > +++ b/src/gen/gen_hdio.c
> > @@ -0,0 +1,337 @@
> > +/* AUTOMATICALLY GENERATED FROM defs/hdio.def - DO NOT EDIT */
>
> The "AUTOMATICALLY GENERATED FROM" part looks too loud,
> does it really need to be written in all caps?
>

I'll sentence case it, and also prepend ./maint/gen to the path so that it
mirrors the warning in generated xlat headers:

    /* Generated by ./maint/gen/generate.sh from ./maint/gen/defs/hdio.def; do not edit. */

> > +#include <stddef.h>
> > +#include "generated.h"
> > +
> > +typedef kernel_ulong_t kernel_size_t;
> > +
> > +#include <linux/hdreg.h>
> > +#include "xlat/hdio_ide_nice.h"
> > +#include "xlat/hdio_busstates.h"
> > +static int
> > +var_leaf_ioctl_HDIO_DRIVE_RESET(struct tcb *tcp, unsigned int code, kernel_ulong_t arg)
> > +{
> > +	tprint_arg_next();
> > +	/* arg: arg (array *) */
> > +	/* using decoder from defs/common.syzlang:26:1 */
> > +
> > +	{
> > +		uint32_t int_buffer;
> > +		print_array(tcp, (arg), (3), &int_buffer, sizeof(int_buffer), tfetch_mem, print_uint32_array_member, 0);
> > +	}
>
> Looks like print_xint32_array_member is a more appropriate way of
> printing
> this data.
>

I'll make this change as well in a v2.