All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Subject: Abuse of warnings for unhandled errors and programming errors
Date: Fri, 08 Aug 2025 11:30:32 +0200	[thread overview]
Message-ID: <87h5yijh3b.fsf@pond.sub.org> (raw)

In "[PATCH 00/12] Error reporting cleanup, a fix, and &error_warn
removal", I challenged the use of warnings in a few places.  I think the
topic deserves a wider audience than the one a rather pedestrian cleanup
series draws.


To make my case, I need to start with errors.  We distinguish between
ordinary errors (for lack of a better word) and programming errors.

Ordinary errors are things like nonsensical user requests, unavailable
resources, and so forth.  A correct program is prepared for such
failures, detects them, and reports them to the user.  The user can then
fix their request, try again when resources are available, and so forth.

Tools for reporting ordinary errors are error_report(),
error_report_err(), &error_fatal, and friends.

Programming errors are bugs.  A developer needs to fix the program.
Unlike ordinary errors, programming errors are *unexpected*.

Programming errors are commonly not recoverable.  The proper tool for
unrecoverable ones is assertions.  &error_abort can be a convenient way
to assert "this can't fail".


On to warnings.

When some failure doesn't prevent satisfying some request, an ordinary
error can be misleading.  We make it a warning instead then.

What if it's a programming error we recover from?

Aside: trying to recover in a buggy program is risky, but that's not the
debate I want to have here.

How do we want such recoverable programming errors reported?

Warning?  We seem to be abusing warnings this way, and I hate it.  What
we have to report is a *bug*, and we should make that crystal clear.
"warning: FunctionYouNeverHeardAbout() failed" does not.  It could be
anything, and you likely need to look at the source to find out.

Ordinary error reporting with "internal error: " prefix, so the user
understands this is a bug, and all they can do about it is report it?

Log the bug somehow?

Thoughts?


Regardless of all that, beware of "this shouldn't fail, and if it does,
I don't know how to handle it / can't be bothered to handle it, so I
just spit out a warning and hope for the best."  Stop and *think*
whether it is a programming error or an ordinary error that is difficult
/ inconvenient / impossible to handle.  Then do the right thing if
practical.  If not, you're accepting a defect.  Make it locally obvious
that it's a defect, so it's at least a *known* defect, and the next guy
coming along won't have to guess.



             reply	other threads:[~2025-08-08  9:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-08  9:30 Markus Armbruster [this message]
2025-08-19 12:30 ` Abuse of warnings for unhandled errors and programming errors Daniel P. Berrangé
2025-09-10 11:05   ` Markus Armbruster
2025-09-15 18:02     ` Daniel P. Berrangé
2025-09-16 13:25       ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h5yijh3b.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.