On Tue, Oct 06, 2015 at 12:45:34AM -0700, Frank Rowand wrote: > On 10/5/2015 9:56 PM, David Gibson wrote: > > On Fri, Oct 02, 2015 at 09:52:48PM -0700, Frank Rowand wrote: > >> From: Frank Rowand <frank.rowand@xxxxxxxxxxxxxx> > >> > >> Proof of concept patch. > >> > >> Annotates input source file and line number of nodes and properties > >> as comments in output .dts file when --annotate flag is supplied. > > < snip > > > > >> Index: b/srcpos.c > >> =================================================================== > >> --- a/srcpos.c > >> +++ b/srcpos.c > >> @@ -246,6 +246,41 @@ srcpos_copy(struct srcpos *pos) > >> return pos_new; > >> } > >> > >> +struct srcpos * > >> +srcpos_copy_all(struct srcpos *pos) > >> +{ > >> + struct srcpos *pos_new; > >> + struct srcfile_state *srcfile_state; > >> + > >> + if (!pos) > >> + return NULL; > >> + > >> + pos_new = srcpos_copy(pos); > >> + > >> + if (pos_new) { > >> + /* allocate without free */ > >> + srcfile_state = xmalloc(sizeof(struct srcfile_state)); > >> + memcpy(srcfile_state, pos->file, sizeof(struct srcfile_state)); > >> + > >> + pos_new->file = srcfile_state; > >> + } > >> + > >> + return pos_new; > >> +} > > > > You still don't need this function. srcfile_states have unlimited > > lifetime. > > My observation about this is buried in a late reply to v5, so > copying here: > > If I don't allocate a new copy of pos->file, then the file names are > incorrect. I'm not sure why. I can dig into this deeper to try to > understand what is going on if you want me to. > > It sounds like I do need to debug this. That's weird. Yeah, we need to debug this. Btw, I was thinking about the filenames - in particular the way the current draft will give you very unwield full paths. I was thinking about a different approach which should shorten those without requiring different invocation or hand hacking: * For the "base" dts file (the one passed on the command line), always use just the basename (i.e. final piece of the file path) * For all other files, print the relative path from the directory of the base file /include/ directives already open files relative to the directory of the file including the /include/ so we have some of the mechanisms for this already. I think this will require a moderate amount of extra work, so I wouldn't suggest it for the first cut, but does it sound like a good approach in principle to you? -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature