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

[Sahil]

Hi,

Thank you for the review.

On Sunday, August 4, 2024 4:59:33 PM GMT+5:30 Dmitry V. Levin wrote:
> 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.

Right, I'll change this.

> > [...]
> > +                      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.

Understood, I'll change this too.

> > [...]
> > +              case WDIOC_SETOPTIONS:
> > +                     if (!umove_or_printaddr(tcp, arg, &options)) {
> > +                            tprint_arg_next();
> 
> Shouldn't this tprint_arg_next() invocation precede umove_or_printaddr()?

Sorry, I forgot to shift this above the "umove".

> > +			tprint_indirect_begin();
> > +			printflags64(watchdog_ioctl_setoptions, options, "WDIOS_???");
> 
> It's an int, why 64?  Just printflags would do.

Got it. I'll change this.

Thanks,
Sahil