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-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
> >>
> >> 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