Re: [PATCH i2c-tools v3 2/2] i2ctransfer: Add optional message modifier flags

["Benoît Monin" <benoit.monin@bootlin.com>]

Hi Wolfram,

On Monday, 8 June 2026 at 11:21:30 CEST, Wolfram Sang wrote:
> > @@ -202,6 +202,20 @@ static int add_flag_if_supported(__u16 *flags, unsigned long funcs, char arg)
> >                         break;
> >  #endif
> >  
> > +               case '0':
> > +               case '1':
> > +               case '2':
> > +               case '3':
> > +               case '4':
> > +               case '5':
> > +               case '6':
> > +               case '7':
> > +               case '8':
> > +               case '9':
> > +               case '?':
> > +               case '@':
> > +                       return 0;
> > +
> >                 default:
> > 
> > 
> > With this we get out of the flags parsing and the "Missing direction flag"
> > error message is shown. What's your opinion on this?
> 
> Basically fine with me, but we need another adaption then because this
> documentation is not precise anymore:
> 
> + * 0 if the argument is a valid and supported optional flag
> 
> Doesn't fit anymore because for the above cases, the argument is not
> valid.
> 
> I think we should return a dedicated value for this 'unexpected' case.
> 
> Which brings something again to the table I was close to suggesting last
> time already but back then refrained from doing so. Now, it might be
> good after all:
> 
> It is about not using magic numbers for the return values of that
> function but an enum with descriptive entries. Example:
> 
> enum supported_flags_retval {
> 	FLAGS_GOT_RW,
> 	FLAGS_GOT_OPTIONAL,
> 	FLAGS_UNEXPECTED,
> 	FLAGS_UNKNOWN,
> };
> 
> I think this will make life for future hackers quite easier. What do you
> think?
> 
Good idea! I'll update to patch and resend.

Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com