Hi Aaron,

On Wed, Apr 10, 2024 at 04:43:53PM -0400, Aaron Merey wrote:
> > > -  /* Pass the file data structure to the caller.  */
> > > -  if (filesp != NULL)
> > > -    *filesp = files;
> > > +      const char **newdirs = (void *) &newfiles->info[nnewfiles];
> > > +      const char **prevdirs = (void *) &prevfiles->info[nprevfiles];
> > > +
> > > +      /* Copy prevdirs to newdirs.  */
> > > +      for (size_t n = 0; n < ndirs; n++)
> > > +     newdirs[n] = prevdirs[n];
> >
> > Again slightly off indentation.
> > But I also don't fully follow the prevdirs/newdirs copying.
> > Why is this? No newdirs are defined here, are there?
> > Maybe I don't understand the data-structure used here.
> 
> The directories are the same but we still need to copy them so that
> dwarf_getsrcdirs can find newfiles' dir names.
> 
> Dwarf_Files is an unusual structure since it doesn't contain a member
> specifically for the array of dirnames.  Instead they're stored at
> the end of the Dwarf_Fileinfo array member.

Aha. Thanks for explaining. Clearly I should not try to review code if
I don't understand the underlying data structures.

> > So testfile-define-file is actually testfile36.debug but with a new
> > line program? How did you edit/insert that one?
> 
> I used xxd to create a hexdump of testfile36.debug and modified the line
> program by hand with a text editor.  I converted the modified hexdump
> back to a binary with xxd -r.
> 
> I chose testfile36.debug because its .debug_line includes multiple
> directory and file names.  It also contains a single short line program
> that was easy to replace with two DW_LNE_define_file opcodes without
> corrupting things.

Fun. Could you add a small description to tests/run-get-files.sh where
testfile-define-file is use so future hackers know how the file was
created?

All looks good BTW. Please do push (if possible with the above change).

Thanks,

Mark

Reply via email to