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. 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. > > 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. -- Thanks, David / dhildenb
