From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9180 invoked by alias); 8 Mar 2010 23:30:21 -0000 Received: (qmail 9166 invoked by uid 22791); 8 Mar 2010 23:30:19 -0000 X-SWARE-Spam-Status: No, hits=-0.6 required=5.0 tests=AWL,BAYES_00,MSGID_MULTIPLE_AT X-Spam-Check-By: sourceware.org Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.200.152) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 08 Mar 2010 23:30:15 +0000 Received: from baal.u-strasbg.fr (baal.u-strasbg.fr [IPv6:2001:660:2402::41]) by mailhost.u-strasbg.fr (8.14.2/jtpda-5.5pre1) with ESMTP id o28NTo18068411 ; Tue, 9 Mar 2010 00:29:50 +0100 (CET) (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from mailserver.u-strasbg.fr (ms3.u-strasbg.fr [IPv6:2001:660:2402:d::12]) by baal.u-strasbg.fr (8.14.0/jtpda-5.5pre1) with ESMTP id o28NTnZW046825 ; Tue, 9 Mar 2010 00:29:49 +0100 (CET) (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from d620muller (lec67-4-82-230-53-140.fbx.proxad.net [82.230.53.140]) (user=mullerp mech=LOGIN) by mailserver.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id o28NTnwt070625 (version=TLSv1/SSLv3 cipher=RC4-MD5 bits=128 verify=NO) ; Tue, 9 Mar 2010 00:29:49 +0100 (CET) (envelope-from pierre.muller@ics-cnrs.unistra.fr) From: "Pierre Muller" To: "'Joel Brobecker'" Cc: References: <001801cabee0$31499ca0$93dcd5e0$@muller@ics-cnrs.unistra.fr> <20100308185450.GK3081@adacore.com> In-Reply-To: <20100308185450.GK3081@adacore.com> Subject: RE: [PATCH] avoid GDB crash on inspection of pascal arrays Date: Mon, 08 Mar 2010 23:30:00 -0000 Message-ID: <001201cabf17$43e1b960$cba52c20$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable 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: 2010-03/txt/msg00346.txt.bz2 > -----Message d'origine----- > De=A0: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] De la part de Joel Brobecker > Envoy=E9=A0: Monday, March 08, 2010 7:55 PM > =C0=A0: Pierre Muller > Cc=A0: gdb-patches@sourceware.org > Objet=A0: Re: [PATCH] avoid GDB crash on inspection of pascal arrays >=20 > Hi Pierre, >=20 > > 2020-03-08 Pierre Muller > > > > * p-lang.c (is_pascal_string_type): Check that TYPE arg is non > NULL. >=20 > Seems odd that you'd call a function whose job is to inspect a type > with > a NULL type, but it's not hard to add a check indeed - and that would > not. > be the first time ;-). Please do not consider this an objection, just > "speaking" aloud... No, you are perfectly right that it doesn't seem logical to call this function with a NULL type. I fact, that happen probably only because of the p-exp.y=20 code, but I tought that adding this test was the safest anyhow. =20 > Just one nit: >=20 > > - if (TYPE_CODE (type) =3D=3D TYPE_CODE_STRUCT) > > + if ((type !=3D NULL) && (TYPE_CODE (type) =3D=3D TYPE_CODE_STRUCT)) >=20 > Would you mind removing the extra parenthesis around each block? Sorry, I am always confused about operator precedence here (pascal 'AND' binary operator has a higher precedence than comparison operators, which means that similar pascal code means those extra parentheses...) > I'd like for the code to be as consistent as possible, to help > readability. It's a question of taste, and I don't agree with all > our rules, but I'd like for things to stay as consistent as possible... >=20 > While I'm sending you an email, I started looking at the call sites > for your function, to see if I could see why the function is called > with a NULL pointer, in case there was something obvious to be found. > Nothing obvious, but I noticed that some code in p-valprint might need > a little reformatting? >=20 > > elttype =3D check_typedef (TYPE_TARGET_TYPE (type)); > > { > > addr =3D unpack_pointer (type, valaddr + embedded_offset); > > print_unpacked_pointer: > > elttype =3D check_typedef (TYPE_TARGET_TYPE (type)); > > > > if (TYPE_CODE (elttype) =3D=3D TYPE_CODE_FUNC) >=20 > (this is around line 153). >=20 > Something else that caught my attention, as well, is the following > statement is repeated twice: >=20 > elttype =3D check_typedef (TYPE_TARGET_TYPE (type)); >=20 > It looks like the first instance is really unnecessary now? > (I am guessing there was a "if" before the mis-indented curly > brace before, and that this "if" got removed, but not its body, > to keep the patch readable - although there is always the diff -w > option). How about the curly brace themselves - since the block > does not introduce new local variables, it looks like it can go too. Formatting with the tab/spaces conversion is still a nightmare for me... I really don't know vi enough to reformat correctly an almost 100 lines long block... Is there a neat way to do this just with vi or do I need something more powerful? Could someone tell me the best way? Pierre