From: Joel Brobecker <brobecker@adacore.com>
To: Nick Roberts <nickrob@snap.net.nz>
Cc: gdb-patches@sourceware.org
Subject: Re: Regression in exec.c (print_section_info) ?
Date: Thu, 01 May 2008 18:07:00 -0000 [thread overview]
Message-ID: <20080501180625.GC3801@adacore.com> (raw)
In-Reply-To: <18454.51085.276103.301144@kahikatea.snap.net.nz>
[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]
> > I would also like to see a new test if we don't already have one to
> > prevent this type of regression in the future.
>
> It's easy to create a test for this specific case but I think it's
> very hard to test the exact format of all CLI output in general, and
> probably not worth the effort. The next regression will likely occur
> elsewhere.
I disagree.
We already do a lot of testing of CLI output, and even if we didn't,
we have to start somewhere. "The next regression will likely occur
elsewhere" might be true, but that doesn't rule out the possibility
of regressing again. Since it is easy to create a test, why not just
do it, even if you think it's not going to bring actual gain? I like
to think of it as insurance. I hope that it will prove useless, but
it's still there if I need it.
> I only have one architecture: my old PC.
That's perfect.
> I didn't really think of it as a patch, I was just trying to draw
> attention to the regression.
This approach is too casual for me. Everyone else writes a ChangeLog,
which takes at most 1 min of your time. Please follow this part of
the procedure, so that I can review the ChangeLog entry as well.
> Actually it was a context diff - so you can read them! They just look
> similar when lines are added.
Argh. Consider sending unified diffs, I think others prefer them too.
When I see context diffs, I usually end up converting it to unified...
Anyway, I ended up checking the attached:
2008-05-01 Nick Roberts <nickrob@snap.net.nz>
* exec.c (print_section_info): Add missing '\n'.
2008-05-01 Joel Brobecker <brobecker@adacore.com>
* gdb.base/info-target.exp: New testcase.
Tested on x86-linux. The new testcase fails without the change to exec.c.
--
Joel
[-- Attachment #2: exec.c.diff --]
[-- Type: text/plain, Size: 839 bytes --]
Index: exec.c
===================================================================
RCS file: /cvs/src/src/gdb/exec.c,v
retrieving revision 1.73
retrieving revision 1.74
diff -u -p -r1.73 -r1.74
--- exec.c 19 Apr 2008 02:07:19 -0000 1.73
+++ exec.c 1 May 2008 17:46:32 -0000 1.74
@@ -543,10 +543,8 @@ print_section_info (struct target_ops *t
wrap_here (" ");
printf_filtered (_("file type %s.\n"), bfd_get_target (abfd));
if (abfd == exec_bfd)
- {
- printf_filtered (_("\tEntry point: "));
- fputs_filtered (paddress (bfd_get_start_address (abfd)), gdb_stdout);
- }
+ printf_filtered (_("\tEntry point: %s\n"),
+ paddress (bfd_get_start_address (abfd)));
for (p = t->to_sections; p < t->to_sections_end; p++)
{
printf_filtered ("\t%s", hex_string_custom (p->addr, wid));
[-- Attachment #3: info-target.exp --]
[-- Type: text/plain, Size: 1471 bytes --]
# Copyright 2008 Free Software Foundation, Inc.
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
if $tracelevel then {
strace $tracelevel
}
set prms_id 0
set bug_id 0
set testfile start
set srcfile ${testfile}.c
set binfile ${objdir}/${subdir}/${testfile}
if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
return -1
}
gdb_exit
gdb_start
gdb_reinitialize_dir $srcdir/$subdir
gdb_load ${binfile}
set eol "\[\r\n\]+"
# Check the output of "info target". Note that we are not interested
# in this case in checking the actual info, but rather to make sure that
# it is formatted properly. For instance, make sure that no '\n' is
# missing at the end some lines.
gdb_test "info target" \
"Symbols from \".*${testfile}.*\"\\..*${eol}Local exec file:${eol}.*Entry point: 0x\[0-9a-zA-Z\]+${eol}.*" \
"info target"
next prev parent reply other threads:[~2008-05-01 18:07 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-04-28 8:38 Nick Roberts
2008-04-29 7:01 ` Joel Brobecker
2008-04-29 14:29 ` Nick Roberts
2008-05-01 18:07 ` Joel Brobecker [this message]
2008-05-02 1:05 ` Nick Roberts
2008-05-02 5:19 ` Joel Brobecker
2008-05-02 22:29 ` Michael Snyder
2008-05-04 8:34 ` Nick Roberts
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080501180625.GC3801@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=nickrob@snap.net.nz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox