From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21831 invoked by alias); 7 Nov 2007 13:16:52 -0000 Received: (qmail 21820 invoked by uid 22791); 7 Nov 2007 13:16:51 -0000 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO brahms.sibelius.xs4all.nl) (82.92.89.47) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 07 Nov 2007 13:16:47 +0000 Received: from brahms.sibelius.xs4all.nl (kettenis@localhost.sibelius.xs4all.nl [127.0.0.1]) by brahms.sibelius.xs4all.nl (8.14.1/8.14.0) with ESMTP id lA7DGg87007479; Wed, 7 Nov 2007 14:16:42 +0100 (CET) Received: (from kettenis@localhost) by brahms.sibelius.xs4all.nl (8.14.1/8.14.1/Submit) id lA7DGgKV018177; Wed, 7 Nov 2007 14:16:42 +0100 (CET) Date: Wed, 07 Nov 2007 13:16:00 -0000 Message-Id: <200711071316.lA7DGgKV018177@brahms.sibelius.xs4all.nl> From: Mark Kettenis To: deuling@de.ibm.com CC: gdb-patches@sourceware.org, uweigand@de.ibm.com In-reply-to: <4731A642.6080402@de.ibm.com> (message from Markus Deuling on Wed, 07 Nov 2007 12:49:22 +0100) Subject: Re: [rfc] [02/05] Get rid of current_gdbarch in hppa-linux-nat.c References: <47319D46.8080904@de.ibm.com> <200711071130.lA7BUDaD014732@brahms.sibelius.xs4all.nl> <4731A642.6080402@de.ibm.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-11/txt/msg00126.txt.bz2 > Date: Wed, 07 Nov 2007 12:49:22 +0100 > From: Markus Deuling > CC: gdb-patches@sourceware.org, uweigand@de.ibm.com > X-XS4ALL-DNSBL-Checked: mxdrop125.xs4all.nl checked 195.212.29.157 against DNS blacklists > X-Virus-Scanned: by XS4ALL Virus Scanner > X-XS4ALL-Spam-Score: -0.0 () SPF_PASS > X-XS4ALL-Spam: NO > X-CNFS-Analysis: v=1.0 c=1 a=oLMpFZsLkpsA:15 a=zG//nGEBJd+KC+fclfqWqQ==:17 a=qtUXupW5FscZJ7vd4pkA:9 a=BxgqAi6GPNXszpZsWqxtEzE25FcA:4 a=vNGxQsTWjH8A:10 > Envelope-To: mark.kettenis@xs4all.nl > X-UIDL: 1194436282._smtp.mxdrop125.51362,S=3200 > > Mark Kettenis schrieb: > >> Date: Wed, 07 Nov 2007 12:11:02 +0100 > >> From: Markus Deuling > >> > >> Hi, > >> > >> this patch adds gdbarch as a parameter to hppa_linux_register_addr. > >> > >> Tested with gdb_mbuild. Ok to commit ? > > > > Actually, this is getting a bit silly. That gdbarch is only needed > > for a sanaity check, and obviously the gdbarch_num_regs call can just > > be replaced with an appropriate bounds check on the u_offsets array. > > > > if (regno < 0 || regno >= ARRAY_SIZE(u_offsets)) > > > > should do the trick. > > Sure it would. But what for do we have gdbarch_num_regs? I dont think its a good idea > to either use gdbarch_num_regs or ARRAY_SIZE(whatever) at will. This is redundant and error-prone. > Btw, there are two further uses of gdbarch_num_regs in that file. > > For my opinion gdbarch should be used to describe an architecture. Indeed, and the check in hppa_linux_register_addr is not checking the architecture. It's there to check whether that function is being called for a register that's outside the range covered by u_offsets. Using gdb_num_regs for that purpose is error-prone, since someone might increase the number of registers for the hppa architecture, but forget to update the u_offsets array. The whole check should probably be converted into a gdb_assert(), since this function should not be called with a register number not covered by u_offsets in the first place. The other uses of gdbarch_num_regs() in this file are fine, since there they are used as the number of registers defined by the architecture. Mark