From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jim Blandy To: Eli Zaretskii Cc: gdb-patches@sources.redhat.com Subject: Re: PATCH: minor cleanup to dwarf2read.c Date: Wed, 04 Jul 2001 01:28:00 -0000 Message-id: References: X-SW-Source: 2001-07/msg00040.html Eli Zaretskii writes: > On Tue, 3 Jul 2001, Jim Blandy wrote: > > > 2001-07-03 Jim Blandy > > > > * 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?