From: Stefan Sperling Subject: Re: diff: handle missing new lines in trailing context To: Tom Jones Cc: gameoftrees@openbsd.org Date: Tue, 4 Oct 2022 12:32:41 +0200 On Fri, Sep 30, 2022 at 02:05:50PM +0100, Tom Jones wrote: > On Thu, Sep 29, 2022 at 12:37:46PM +0200, Stefan Sperling wrote: > > On Thu, Sep 29, 2022 at 12:24:22PM +0200, Stefan Sperling wrote: > > > Well, our loops certainly take great care not to index result->chunks.head > > > in the way you are proposing. For example: > > > > > > for (c_idx = cc->chunk.start; c_idx < cc->chunk.end; c_idx++) { > > > const struct diff_chunk *c = &result->chunks.head[c_idx]; > > > > > > This keeps c_idx < chunk.end, not <= chunk.end. > > > > > > Can you show what you are seeing in the debugger? What are the contents > > > of the final chunk? In particular, what is that chunk's type? > > > > Some more hints: > > > > Keep in mind that an arraylist might have more memory allocated than > > needed to represent the elements of the array (the elements being diff > > chunks in this case). > > > > What is important is that we never index at or beyond the chunk arraylist's > > len field. If there is a chunk at index 'len' it should not be considered > > valid. Because it is not part of the user-visible data stored on the array. > > > > The 'chunk.end' index points to the first chunk array element following this > > chunk. If chunk.end equals the chunk arraylist's len, we are at end-of-file. > > Is the correct final chunk in a result the one at result->chunks.len - > 1? > > This works: > rc = diff_output_trailing_newline_msg(outinfo, dest, > &result->chunks.head[result->chunks.len - 1]); > > and doesn't violate the public length of the array list. Yes, this looks correct to me. If this passes your test then it should be the right way to fix the issue. ok stsp@