From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40834) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z89uA-0006RD-RH for qemu-devel@nongnu.org; Thu, 25 Jun 2015 12:21:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z89u7-0006x4-KC for qemu-devel@nongnu.org; Thu, 25 Jun 2015 12:21:34 -0400 Received: from cantor2.suse.de ([195.135.220.15]:56711 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z89u7-0006wy-Dx for qemu-devel@nongnu.org; Thu, 25 Jun 2015 12:21:31 -0400 Message-ID: <558C2A8A.8070202@suse.de> Date: Thu, 25 Jun 2015 18:21:30 +0200 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <06c2a347f5ae9b1189cdfceeca8f885bc992c1e7.1435115710.git.crosthwaite.peter@gmail.com> <558AF25D.4070106@suse.de> <558BE212.10802@suse.de> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH qom v3 1/4] cpu: Add wrapper to the set-pc() hook List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: "Edgar E. Iglesias" , Peter Crosthwaite , QEMU Developers , Peter Crosthwaite Am 25.06.2015 um 13:21 schrieb Peter Maydell: > On 25 June 2015 at 12:12, Andreas F=C3=A4rber wrote: >> Am 24.06.2015 um 21:11 schrieb Peter Maydell: >>> On 24 June 2015 at 19:09, Andreas F=C3=A4rber wrot= e: >>>> + g_assert(cc->set_pc !=3D NULL); >>>> + cc->set_pc(cpu, addr); >>>> } >>> >>> Do we need this assert? If it would have fired >>> then we'll just crash immediately calling the null pointer, >>> so it's not like it's guarding against a more subtle failure >>> at a later point... >> >> There seemed uncertainty whether all corner cases of the 17 targets >> implement set_pc for all subclasses. By my reading, g_assert() calls >> g_assertion_message_expr(), which is marked G_GNUC_NORETURN - and I >> assume it to abort after printing the message, raising a signal and >> either exiting the process or falling back to an attached debugger. >> >> It may be unnecessary, but I don't see it calling the null pointer her= e. >=20 > What I mean is: > * with the assert, QEMU will die in this function if cc->set_pc > is NULL, in an easily debuggable way > * without the assert, QEMU will still die in this function if > cc->set_pc is NULL, in an easily debuggable way >=20 > So the assert doesn't hurt, but it doesn't really gain anything IMHO. > Assertions are most useful when they turn something that would be > a really confusing failure much later in execution into an easily > debuggable crash earlier on, I think. >=20 > Still, this is a very minor thing, so it's a personal taste/style > question, as you say. You can leave the assert in or remove it, > whichever you prefer. Okay, dropping the assertion: diff --git a/include/qom/cpu.h b/include/qom/cpu.h index e7ee6c6..5db1ea3 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -610,7 +610,6 @@ static inline void cpu_set_pc(CPUState *cpu, vaddr ad= dr) { CPUClass *cc =3D CPU_GET_CLASS(cpu); - g_assert(cc->set_pc !=3D NULL); cc->set_pc(cpu, addr); } Cheers, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Felix Imend=C3=B6rffer, Jane Smithard, Dilip Upmanyu, Graham Norton; = HRB 21284 (AG N=C3=BCrnberg)