On 9/10/20 8:35 AM, Martin Kletzander wrote:
On Thu, Sep 10, 2020 at 01:56:53PM +0200, Pino Toscano wrote:
A lot of virReportError() calls use VIR_ERR_INTERNAL_ERROR to represent
the number of the error, even in cases where there is one fitting more.
Hence, replace some of them with better virErrorNumber values.
This is something that we need to do in oh-so-many places, yes.
I don't disagree with you, but as a tempering comment to this, I've
always thought of INTERNAL_ERROR as a way to mark something that should
never happen, and if it did it's due to the way the code is written, not
the fault of the user. So there could be places where, e.g. yes, the
argument is invalid, but the invalid argument should have been weeded
out before this point, or it was selected by the software and not the
user - basically it's a way of saying "This should never happen, and if
it didn't, some programmer has some 'splaining to do."
I guess I'm just saying that any change would need to take into account
the larger context around the error, not just the few lines where the
error is detected and logged.
(NB: I didn't even look at Pino's changes to see which ones he changed;
just wanted to get my philosophy out there)
(NB2: still the most important part of any error message (for us) is
knowing the exact line where the error occurred, and the values of any
pertinent variables that caused it; for me the type of error (which is
in many cases a touchy feely thing rather than hard fact) is secondary)
