Re: PATCH: minor cleanup to dwarf2read.c

[Jim Blandy <jimb at zwingli dot cygnus dot com>]

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?