On Sunday, December 1, 2019, Aleksandar Markovic < aleksandar.m.m...@gmail.com> wrote:
> > > On Sunday, December 1, 2019, Aleksandar Markovic < > aleksandar.m.m...@gmail.com> wrote: > >> >> >> On Saturday, November 30, 2019, David Hildenbrand <dhild...@redhat.com> >> wrote: >> >>> >>> >>> > Am 30.11.2019 um 20:42 schrieb Markus Armbruster <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 >>> >>> „ 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. >> >> > Oops, I didn't even realize that the patch title contains the word > "latent". (I wrote the previous message without that knowledge. For some > strange reason, my email client doesn't display email subject while > replying.) > > In this case, I would suggest usage of phrase "latent bug" instead of > "latent error" or so in the message title, to strenghten the point that > this is not a cleanup. > > Actually, the message title already does use "latent .... bugs". So it is fine - in my opinion. > Yours, Aleksandar > > > >> 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. >> >> David, if I understand well, this patch fixes the commit done by you. I >> definitely understand this is not a pleasant position, but we all >> (definitelly including myself too) should learn to handle such situations >> as gracefully as we can. >> >> Yours, >> Aleksandar >> >> >> >>> >>> >>> >>> Also, please change the terminology „messed up“ to „introduced in“ or >>> similar. >>> >>> (applies to all s390x patches) >>> >>> Thanks. >> >>
