From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28969 invoked by alias); 6 Aug 2010 20:53:05 -0000 Received: (qmail 28956 invoked by uid 22791); 6 Aug 2010 20:53:04 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_PASS,TW_BJ,TW_JC,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.44.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 06 Aug 2010 20:52:54 +0000 Received: from hpaq2.eem.corp.google.com (hpaq2.eem.corp.google.com [172.25.149.2]) by smtp-out.google.com with ESMTP id o76Kqqsi014972 for ; Fri, 6 Aug 2010 13:52:52 -0700 Received: from vws3 (vws3.prod.google.com [10.241.21.131]) by hpaq2.eem.corp.google.com with ESMTP id o76Kqldu018753 for ; Fri, 6 Aug 2010 13:52:51 -0700 Received: by vws3 with SMTP id 3so8452965vws.33 for ; Fri, 06 Aug 2010 13:52:50 -0700 (PDT) MIME-Version: 1.0 Received: by 10.220.75.148 with SMTP id y20mr8668839vcj.144.1281127970739; Fri, 06 Aug 2010 13:52:50 -0700 (PDT) Received: by 10.220.201.197 with HTTP; Fri, 6 Aug 2010 13:52:50 -0700 (PDT) In-Reply-To: References: Date: Fri, 06 Aug 2010 20:53:00 -0000 Message-ID: Subject: Re: [0/4] RFC: add DWARF index support From: Doug Evans To: Tom Tromey Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-System-Of-Record: true X-IsSubscribed: yes 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-08/txt/msg00087.txt.bz2 On Fri, Aug 6, 2010 at 10:39 AM, Tom Tromey wrote: > Doug> IWBN to add to the comment about exiting without creating an index = not > Doug> being an error, e.g. provide an example. > Doug> Is it because the file could be stripped? =A0[If it is stripped, sh= ould > Doug> the script fail or pass? =A0Dunno.] > > I don't want to do this, because the reasons may change. I don't want to unnecessarily delay the patch (given, for example, its size), but not generating an index file when asked to and not flagging an error feels wrong at a gut level. OTOH strip doesn't flag an error if the file is already stripped. Maybe this is akin to that. At any rate, the some examples of this behaviour may be things the user could trip over, so we should document at least those reasons somewhere, the script could just contain a reference to the docs. E.g., I wonder what should happen if there isn't enough (or any) info to generate .gdb-index. Is that an error? I just tried it, no index is saved and gdb doesn't flag an error. I'm not sure I like that behaviour (but I wouldn't hold up the patch because of it, I think we agree that the index generation should be done elsewhere anyway), but can we agree that if we keep it it needs to be documented (somewhere). ["it" being cases like trying to generate an index when there is no debug info present to begin with, and anything else the user might easily trip over.] [For completeness sake ... OTOH, given that index generation should be done elsewhere anyway, I'm more inclined to be rather strict with what is successful behaviour. And if such things did flag an error, I think there's less need to document them. :-)] > Doug> IWBN to put "${file}.gdb-index" in its own variable so that there's > Doug> just one instance. > > Ok. > > Doug> LGTM with the above nits. > > I don't know what LGTM means. "Looks good to me." [for reference sake, I did see where it ranked in google's search results before using it. 1/2 :-) I wasn't sure how common its use was (outside of google anyway).] > How about this? > > Tom > > 2010-08-05 =A0Tom Tromey =A0 > > =A0 =A0 =A0 =A0* gdb-add-index.sh: Add error checking. > > Index: gdb-add-index.sh > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/gdb-add-index.sh,v > retrieving revision 1.1 > diff -u -r1.1 gdb-add-index.sh > --- gdb-add-index.sh =A0 =A030 Jul 2010 20:46:34 -0000 =A0 =A0 =A01.1 > +++ gdb-add-index.sh =A0 =A06 Aug 2010 17:39:20 -0000 > @@ -16,14 +16,29 @@ > =A0# You should have received a copy of the GNU General Public License > =A0# along with this program. =A0If not, see . > > +if test $# -ne 1; then > + =A0 echo "Usage: gdb-add-index FILE" 1>&2 > + =A0 exit 1 > +fi > + > =A0file=3D"$1" > =A0dir=3D"${file%/*}" > +index=3D"${file}.gdb-index" > > -gdb --batch-silent -ex "file $file" -ex "save gdb-index $dir" > +gdb --batch-silent -ex "file $file" -ex "save gdb-index $dir" || { > + =A0 status=3D$? > + =A0 # Just in case. > + =A0 rm -f "$index" > + =A0 exit $status > +} > > -if test -f "${file}.gdb-index"; then > - =A0 objcopy --add-section .gdb_index=3D"${file}.gdb-index" --set-sectio= n-flags .gdb_index=3Dreadonly "$file" "$file" > - =A0 rm -f "${file}.gdb-index" > +# In some situation gdb can exit without creating an index. =A0This is > +# not an error. > +status=3D0 > +if test -f "${index}"; then > + =A0 objcopy --add-section .gdb_index=3D"${index}" --set-section-flags .= gdb_index=3Dreadonly "$file" "$file" > + =A0 status=3D$? > + =A0 rm -f "${index}" > =A0fi > > -exit 0 > +exit $status > Ok. I hate to add another wrinkle, and thanks for bearing with me. I just tried passing a file that doesn't exist. gdb writes something to stderr and then exits with a zero exit code. That needs to be flagged as an error (presumably with test -r or some such before invoking gdb) , but it could be deferred to another patch.