[RFC/PATCH] watchdog_ioctl: Add decoding of WDIOC_GETSUPPORT and WDIOC_SETOPTIONS ioctls.

Dmitry V. Levin ldv at strace.io
Sun Aug 4 11:29:33 UTC 2024


Hi,

On Sun, Aug 04, 2024 at 12:28:00PM +0530, Sahil Siddiq wrote:
[...]
> @@ -17,7 +20,30 @@ int
>  watchdog_ioctl(struct tcb *const tcp, const unsigned int code,
>  	   const kernel_ulong_t arg)
>  {
> +	struct watchdog_info ident;
> +	int options;

Given that "options" are flags, "unsigned int" would suit it better.

>  	switch (code) {
> +	case WDIOC_GETSUPPORT:
> +		if (entering(tcp))
> +			return 0;
> +		tprint_arg_next();
> +		if (!umove_or_printaddr(tcp, arg, &ident)) {
> +			tprint_struct_begin();
> +			PRINT_FIELD_FLAGS(ident, options, watchdog_ioctl_support_options,
> +					  "WDIOF_???");
> +			tprint_struct_next();
> +			if (abbrev(tcp))
> +				tprint_more_data_follows();
> +			else {
> +				PRINT_FIELD_U(ident, firmware_version);
> +				tprint_struct_next();
> +				tprints_field_name("identity");
> +				printstr(tcp, arg + offsetof(struct watchdog_info, identity));

Since identity is a part of struct watchdog_info, it's already fetched,
so this field can be printed using PRINT_FIELD_CSTRING.

> +			}
> +			tprint_struct_end();
> +		}
> +		break;
>  	case WDIOC_GETSTATUS:
>  	case WDIOC_GETBOOTSTATUS:
>  	case WDIOC_GETTEMP:
> @@ -32,6 +58,13 @@ watchdog_ioctl(struct tcb *const tcp, const unsigned int code,
>  		tprint_arg_next();
>  		printnum_int(tcp, arg, "%d");
>  		break;
> +	case WDIOC_SETOPTIONS:
> +		if (!umove_or_printaddr(tcp, arg, &options)) {
> +			tprint_arg_next();

Shouldn't this tprint_arg_next() invocation precede umove_or_printaddr()?

> +			tprint_indirect_begin();
> +			printflags64(watchdog_ioctl_setoptions, options, "WDIOS_???");

It's an int, why 64?  Just printflags would do.


-- 
ldv


More information about the Strace-devel mailing list