On Thu, May 2, 2024 at 9:40 PM Marek Behún <kabel@xxxxxxxxxx> wrote:
> On Tue, Apr 30, 2024 at 06:17:45PM +0300, Andy Shevchenko wrote:
> > On Tue, Apr 30, 2024 at 04:05:07PM +0200, Marek Behún wrote:
> > > On Tue, 30 Apr 2024 15:53:51 +0300
> > > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> > > > On Tue, Apr 30, 2024 at 2:51 PM Marek Behún <kabel@xxxxxxxxxx> wrote:
...
> > > > > +static int omnia_get_version_hash(struct omnia_mcu *mcu, bool bootloader,
> > > > > + u8 version[static OMNIA_FW_VERSION_HEX_LEN])
> > > >
> > > > Interesting format of the last parameter. Does it make any difference
> > > > to the compiler if you use u8 *version?
> > >
> > > The compiler will warn if an array with not enough space is passed as
> > > argument.
> >
> > Really?
>
> Indeed:
>
> extern void a(char *x);
>
> static void b(char x[static 10]) {
> a(x);
> }
>
> void c(void) {
> char x[5] = "abcd";
> b(x);
It's not the example I was talking about. Here should be a(x).
> }
>
> says:
> test.c: In function ‘c’:
> test.c:9:9: warning: ‘b’ accessing 10 bytes in a region of size 5 [-Wstringop-overflow=]
> 9 | b(x);
> | ^~~~
> test.c:9:9: note: referencing argument 1 of type ‘char[10]’
> test.c:3:13: note: in a call to function ‘b’
> 3 | static void b(char x[static 10]) {
> |
...
> > > > > + omnia_info_missing_feature(dev, "feature reading");
> > > >
> > > > Tautology. Read the final message. I believe you wanted to pass just
> > > > "reading" here.
> > >
> > > No, I actually wanted it to print
> > > Your board's MCU firmware does not support the feature reading
> > > feature.
> > > as in the feature "feature reading" is not supported.
> > > I guess I could change it to
> > > Your board's MCU firmware does not support the feature reading.
> > > but that would complicate the code: either I would need to add
> > > " feature" suffix to all the features[].name, or duplicate the
> > > info string from the omnia_info_missing_feature() function.
> >
> > From point of a mere user (as I am towards this driver) I consider
> > the tautology confusing.
> >
> > ...the 'reading' feature
> >
> > may be a good compromise.
>
> I have rewritten it to use another dev_info:
> dev_info(dev,
> "Your board's MCU firmware does not support feature reading.\n");
OK! As long as there are no two messages in a row for the same error.
--
With Best Regards,
Andy Shevchenko
