On Wed, Aug 11, 2021 at 02:29:12PM +0200, Wolfgang Denk wrote:
> Dear Rasmus,
>
> In message <3d48015a-07d3-e296-b9ba-a1edd455c...@prevas.dk> you wrote:
> >
> > >> + if (ret) {
> > >> + log_debug("Error getting UCLASS_WDT: %d\n", ret);
> > >
> > > Perhaps log_err()?
> >
> > No, we've already been over this in earlier discussions (it's the exact
> > same pattern and reasoning as initr_watchdog). If I made it log_err(),
> > it would cost .text for something that never-ever happens in practice,
> > while log_debug() is usually a no-op, but can be compiled in if
> > something truly fishy seems to be going on.
>
> This argument fits on all types or effors: they are supposed to
> never ever happen - at least in theory; in reality they do, and more
> often than we like.
>
> And a proper error message is mandatory for correct error handling.Error messages are a fine line to walk. We've got to have been very badly corrupted to go down this error path. There's going to be lots of other error messages popping out. Saving a bit of .text is good. It makes it easier to justify spending a little .text later. > > > Looks good, thanks for quickly working on this. Not sure, if this new > > > function should be "void" or better "int" so that the error can be > > > returned. > > > > That's why I included my tentative commit log, so you could see my > > explanation for why I made it void. Until some user shows up that > > _wants_ a return value, there's no point making it return int. When that > > user shows up, we can discuss which int (return early on failure? > > remember that an error was seen but still call wdt_stop on remaining > > devices? etc. etc.). > > Returning an error code is always a good ide, no matter if > current users check it or not. And here I agree, catch an error code, pass the error code back to the caller. That's far more important than making sure that every error code we catch logs a message by default every time. -- Tom
signature.asc
Description: PGP signature
