This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: PATCH: minor cleanup to dwarf2read.c
- To: Eli Zaretskii <eliz at is dot elta dot co dot il>
- Subject: Re: PATCH: minor cleanup to dwarf2read.c
- From: Jim Blandy <jimb at zwingli dot cygnus dot com>
- Date: 04 Jul 2001 03:29:20 -0500
- Cc: gdb-patches at sources dot redhat dot com
- References: <Pine.SUN.3.91.1010704095837.3231R-100000@is>
Eli Zaretskii <eliz@is.elta.co.il> writes:
> On Tue, 3 Jul 2001, Jim Blandy wrote:
>
> > 2001-07-03 Jim Blandy <jimb@redhat.com>
> >
> > * dwarf2read.c (dwarf2_build_psymtabs_hard): Remove extraneous
> > code in loop condition. This seemed to be trying to round
> > info_ptr up to the next four-byte boundary, but that's not what it
> > actually did. If we discover the problem the old code was really
> > trying to address, we can fix it properly.
>
> IMHO, ChangeLog is never a proper place to put such comments. Should
> the problem surface in the future, how do we expect someone to find
> this piece of info?
>
> I suggest to put this text as a comment in the source, together with a
> copy of the old code, in case someone will actually need to fix
> this.
I did hesitate to put that in the ChangeLog. I didn't because I felt
the code was pretty obvious.
There's a buffer of byte-oriented data, dwarf_info_buffer, whose
length in bytes is dwarf_info_size. We're walking through it with a
char pointer named info_ptr. Each iteration through the loop, we
advance info_ptr over some variable number of bytes. There's no way
clever tests for termination belong in the while condition, since the
things we're reading have non-trivial structure; overrun tests need to
be in the individual functions that consume data and advance info_ptr.
In other words, it was a perfectly straightforward situation marred by
obscure code. I don't think it really needs a comment at all. If you
have:
foo_buf = malloc (sizeof (*foo_buf) * foo_length);
for (i = 0; i < foo_length; i++)
...
you don't put any scary comments around the for statement about the
termination conditions, do you?