Re: [PATCH i2c-tools v4] i2ctransfer: Add optional message modifier flags

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]

Hi Benoît,

Looks mostly good, I think we are nearly there.

> +	/* unexpected next part of the message: length or address */
> +	case '0':
> +	case '1':
> +	case '2':
> +	case '3':
> +	case '4':
> +	case '5':
> +	case '6':
> +	case '7':
> +	case '8':
> +	case '9':
> +	case '?':
> +	case '@':
> +		fprintf(stderr, "Error: Unexpected flag '%c'\n", arg);
> +		return FLAGS_UNEXPECTED;

I suggest to drop this error message. The above characters are not
exactly flags. By removing the printout here...

> +			for (ret = FLAGS_GOT_OPTIONAL; *arg_ptr && ret == FLAGS_GOT_OPTIONAL; arg_ptr++)
> +				ret = add_flag_if_supported(&flags, funcs, *arg_ptr);
> +			if (ret == FLAGS_UNKNOWN || ret == FLAGS_UNEXPECTED || ret == FLAGS_UNSUPPORTED)

... and simplifying this to ...

+			if (ret == FLAGS_UNKNOWN || ret == FLAGS_UNSUPPORTED)

... we get the error message of a missing r/w-flag instead. I think this
is more helpful to the user? What do you think?

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]