Re: Regression in exec.c (print_section_info) ?

Joel Brobecker
2008-05-01T18:07:00Z

[-- 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"