Re: [PATCH v4 07/10] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

Tom Rini
2021-08-11T12:43:33Z

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

On Wed, Aug 11, 2021 at 02:29:12PM +0200, Wolfgang Denk wrote:
> Dear Rasmus,
>
> In message <3d48015a-07d3-e296-b9ba-a1edd455ce9e@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

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