David Hildenbrand <da...@redhat.com> writes:
> On 01.12.19 14:46, Aleksandar Markovic wrote:
>>
>>
>> On Saturday, November 30, 2019, David Hildenbrand <dhild...@redhat.com
>> <mailto:dhild...@redhat.com>> wrote:
>>
>>
>>
>> > Am 30.11.2019 um 20:42 schrieb Markus Armbruster
>> <arm...@redhat.com <mailto:arm...@redhat.com>>:
>> >
>> > cpu_model_from_info() is a helper for
>> qmp_query_cpu_model_expansion(),
>> > qmp_query_cpu_model_comparison(), qmp_query_cpu_model_baseline(). It
>> > crashes when the visitor or the QOM setter fails, and its @errp
>> > argument is null. Messed up in commit 137974cea3 's390x/cpumodel:
>> > implement QMP interface "query-cpu-model-expansion"'.
>> >
>> > Its three callers have the same bug. Messed up in commit 4e82ef0502
>> > 's390x/cpumodel: implement QMP interface "query-cpu-model-comparison"'
>> > and commit f1a47d08ef 's390x/cpumodel: implement QMP interface
>> > "query-cpu-model-baseline"'.
>> >
>> > The bugs can't bite as no caller actually passes null. Fix them
>> > anyway.
>>
>> https://en.m.wikipedia.org/wiki/Software_bug
>> <https://en.m.wikipedia.org/wiki/Software_bug>
>>
>> „ A software bug is an error, flaw or fault in a computer program
>> or system that causes it to produce an incorrect or unexpected
>> result, or to behave in unintended ways. „
>>
>> Please make it clear in the descriptions that these are cleanups and
>> not bugfixes. It might be very confusing for people looking out for
>> real bugs.
>>
>>
>>
>> Disclaimer: I am not entirely familiar with the code in question, so
>> take my opinion with reasonablereservation.
>>
>> It looks that we here deal with latent bugs. As you probably know from
>> experience, a latent bugs, when they are activated with some ostensibly
>> unrelated code change, can be much more difficult to diagnose and fix
>> than regular bugs.
>
> "https://economictimes.indiatimes.com/definition/latent-bug
>
> "Definition: An uncovered or unidentified bug which exists in the system
> over a period of time is known as the Latent Bug. The bug may persist in
> the system in one or more versions of the software."
>
> AFAIK, a latent BUG can be triggered, it simply was never triggered.
First search hit. Here's my second one:
Q: What are latent bugs?
A: These bugs do not cause problems today. However, they are lurking
just waiting to reveal themselves later. The Ariane 5 rocket
failure was caused by a float->int conversion error that lay dormant
when previous rockets were slower; but the faster Ariane 5 triggered
the problem. The Millennium bug is another example of a latent bug
that came to light when circumstances changed. Latent bugs are much
harder to test using conventional testing techniques, and finding
them requires someone with foresight to ask.
http://www.geekinterview.com/question_details/36689
My point is: common usage of the term is not as clear-cut as your quote
makes it seem.
> Do you think the following code is buggy?
>
> static int get_val(int *ptr)
> {
> return *ptr;
> }
>
> int main()
> {
> int a = 0;
>
> return get_val(&a);
> }
>
> I claim, no, although we could access a NULL pointer if ever reworked.
> There is no invalid system state possible.
get_val() is silent on how it wants to be used. error.h is anything
but: it spells out how it wantes to be used in quite some detail. In
particular:
* Receive an error and pass it on to the caller:
* Error *err = NULL;
* foo(arg, &err);
* if (err) {
* handle the error...
* error_propagate(errp, err);
* }
* where Error **errp is a parameter, by convention the last one.
*
* Do *not* "optimize" this to
* foo(arg, errp);
* if (*errp) { // WRONG!
* handle the error...
* }
* because errp may be NULL!
My patch fixes this exact misuse of the interface.
>> In that light, this change is not a clean up. It is a fix of a latent
>> bugs, and Markus' aproach to treat it as a bug fix looks right to me. I
>> would just add a word "latent" or similar, which would even more
>> distance the patch from "cleanup" meaning.
>
> I agree iff there is some way to trigger it. Otherwise, to me it is a
> cleanup.If it's a BUG, it deserves proper Fixes tags and some
> description how it can be triggered.
Yes, a bug that can bite deserves a reproducer and a formal Fixes: tag.
The thing we're discussing (however we may want to call it) does not
have a reproducer, and I think we're in agreement that it doesn't need a
Fixes: tag.
However, my patch is not cleaning up something that's dirty, it's fixing
something that's unequivocally wrong: a violation of a stated interface
contract.
The violation happens to have no ill effects at this time due to the way
the violating code is being used.
I call that a "latent bug". git-log has quite a few occurences of
"latent bug", by Richard Henderson, Daniel Berrangé, Paolo, ...
Your point that the commit message should not confuse people looking for
real bugs is well taken. I think "latent bug" is clear enough, and also
concise. I'm of course open to better phrasings.
s390x: Fix currently harmless query-cpu-model-FOO error API violations
feels no clearer to me than
s390x: Fix latent query-cpu-model-FOO error handling bugs
It's also too long.
I tried. Your turn :)
