On Tue, 2023-08-15 at 13:58 -0400, Lewis Hyatt wrote: > On Tue, Aug 15, 2023 at 11:43:05AM -0400, David Malcolm wrote: > > On Wed, 2023-08-09 at 18:14 -0400, Lewis Hyatt wrote: > > > Class file_cache_slot in input.cc is used to query specific lines > > > of source > > > code from a file when needed by diagnostics infrastructure. This > > > will be > > > extended in a subsequent patch to support obtaining the source > > > code from > > > in-memory generated buffers rather than from a file. The present > > > patch > > > refactors class file_cache_slot, putting most of the logic into a > > > new base > > > class cache_data_source, in preparation for reusing that code in > > > the next > > > patch. There is no change in functionality yet. > > >
[...snip...] > > > > I confess I had to reread both this and patch 4/8 to make sense of > > this; this is probably one of those cases where it's harder to read > > in > > patch form than as source, but I think I now understand the new > > implementation. > > Yes, sorry about that. I hope at least splitting into two patches > here made it > a little easier. > > > > > Did you try testing this with valgrind (e.g. "make selftest- > > valgrind")? > > > > Oh interesting, was not aware of this. I think it shows that new > leaks were > not introduced with the patch series. > [...snip...] > > > > I don't think we have any selftest coverage for "\r" in the line- > > break > > handling; that would be good to add. > > > > This patch is OK for trunk once the rest of the kit is approved. > > Thank you. To be clear, were you suggesting to add selftest coverage > for \r > endings now, or in a follow up? The former, please, so that we can sure that the patch doesn't introduce any buffer overreads etc. Thanks Dave
