On Fri, 20 Apr 2018, David Gibson wrote:
> On Fri, Feb 02, 2018 at 09:41:50PM +0100, Julia Lawall wrote:
> > This commit provides two new command-line options:
> >
> > --annotate (abbreviated -T)
> > --annotate-full (abbreviated -F)
>
> Rather than two new options, can I suggest an "annotate level". So -T
> would give you the brief ones, -T -T would give you the full ones
> (similar to the way -q works).
Good idea, thanks. I'll try to address this and the other comments in the
next couple of days.
julia
>
> > --annotate provides one or more filenames and line numbers indicating
> > the origin of a given line. The filename is expressed relative the the
> > filename provided on the command line. Nothing is printed for overlays,
> > etc.
> >
> > --annotate-full provides one or more tuples of: filename, starting line,
> > starting column, ending line ending column. The full path is given for
> > the file name. Overlays, etc are annotated with <no-file>:<no-line>.
> >
> > --annotate-full may be too verbose for normal use.
> >
> > There are numerous changes in srcpos to provide the relative filenames
> > (variables initial_path, initial_pathlen and initial_cpp, new functions
> > set_initial_path and shorten_to_initial_path, and changes in
> > srcfile_push and srcpos_set_line). The change in srcpos_set_line takes
> > care of the case where cpp is used as a preprocessor. In that case the
> > initial file name is not the one provided on the command line but the
> > one found at the beginnning of the cpp output.
> >
> > The new functions srcpos_string_comment, srcpos_string_first, and
> > srcpos_string_last print the annotations. srcpos_string_comment is
> > recursive to print a list of source file positions.
> >
> > Various changes are sprinkled throughout treesource.c to print the
> > annotations.
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@xxxxxxx>
> > Signed-off-by: Frank Rowand <frank.rowand@xxxxxxxx>
> > ---
> > v4: no change
> >
> > dtc.c | 19 ++++++++-
> > dtc.h | 2 +
> > srcpos.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
> > srcpos.h | 2 +
> > treesource.c | 47 +++++++++++++++++++---
> > 5 files changed, 176 insertions(+), 18 deletions(-)
> >
> > diff --git a/dtc.c b/dtc.c
> > index c36994e..10786a4 100644
> > --- a/dtc.c
> > +++ b/dtc.c
> > @@ -35,6 +35,8 @@ int phandle_format = PHANDLE_EPAPR; /* Use linux,phandle or phandle properties *
> > int generate_symbols; /* enable symbols & fixup support */
> > int generate_fixups; /* suppress generation of fixups on symbol support */
> > int auto_label_aliases; /* auto generate labels -> aliases */
> > +bool annotate; /* =false, annotate .dts with input source location */
> > +bool annotate_full; /* =false, annotate .dts with full input source location */
> >
> > static int is_power_of_2(int x)
> > {
> > @@ -60,7 +62,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
> >
> > /* Usage related data. */
> > static const char usage_synopsis[] = "dtc [options] <input file>";
> > -static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@Ahv";
> > +static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E:@ATFhv";
> > static struct option const usage_long_opts[] = {
> > {"quiet", no_argument, NULL, 'q'},
> > {"in-format", a_argument, NULL, 'I'},
> > @@ -81,6 +83,8 @@ static struct option const usage_long_opts[] = {
> > {"error", a_argument, NULL, 'E'},
> > {"symbols", no_argument, NULL, '@'},
> > {"auto-alias", no_argument, NULL, 'A'},
> > + {"annotate", no_argument, NULL, 'T'},
> > + {"annotate-full", no_argument, NULL, 'F'},
> > {"help", no_argument, NULL, 'h'},
> > {"version", no_argument, NULL, 'v'},
> > {NULL, no_argument, NULL, 0x0},
> > @@ -114,6 +118,8 @@ static const char * const usage_opts_help[] = {
> > "\n\tEnable/disable errors (prefix with \"no-\")",
> > "\n\tEnable generation of symbols",
> > "\n\tEnable auto-alias of labels",
> > + "\n\tAnnotate output .dts with input source file and line",
> > + "\n\tAnnotate output .dts with input source file (full path) line and column",
> > "\n\tPrint this help and exit",
> > "\n\tPrint version and exit",
> > NULL,
> > @@ -259,6 +265,13 @@ int main(int argc, char *argv[])
> > case 'A':
> > auto_label_aliases = 1;
> > break;
> > + case 'F':
> > + annotate = true;
> > + annotate_full = true;
> > + break;
> > + case 'T':
> > + annotate = true;
> > + break;
> >
> > case 'h':
> > usage(NULL);
> > @@ -297,6 +310,10 @@ int main(int argc, char *argv[])
> > outform = "dts";
> > }
> > }
> > +
> > + if (annotate && (!streq(inform, "dts") || !streq(outform, "dts")))
> > + die("--annotate and --annotate-full require -I dts -O dts\n");
> > +
> > if (streq(inform, "dts"))
> > dti = dt_from_source(arg);
> > else if (streq(inform, "fs"))
> > diff --git a/dtc.h b/dtc.h
> > index 42106c2..63ba433 100644
> > --- a/dtc.h
> > +++ b/dtc.h
> > @@ -58,6 +58,8 @@ extern int phandle_format; /* Use linux,phandle or phandle properties */
> > extern int generate_symbols; /* generate symbols for nodes with labels */
> > extern int generate_fixups; /* generate fixups */
> > extern int auto_label_aliases; /* auto generate labels -> aliases */
> > +extern bool annotate; /* annotate .dts with input source location */
> > +extern bool annotate_full; /* annotate .dts with detailed source location */
> >
> > #define PHANDLE_LEGACY 0x1
> > #define PHANDLE_EPAPR 0x2
> > diff --git a/srcpos.c b/srcpos.c
> > index c6688e7..916dc8d 100644
> > --- a/srcpos.c
> > +++ b/srcpos.c
> > @@ -33,6 +33,9 @@ struct search_path {
> > /* This is the list of directories that we search for source files */
> > static struct search_path *search_path_head, **search_path_tail;
> >
> > +/* Detect infinite include recursion. */
> > +#define MAX_SRCFILE_DEPTH (100)
> > +static int srcfile_depth; /* = 0 */
> >
> > static char *get_dirname(const char *path)
> > {
> > @@ -51,11 +54,51 @@ static char *get_dirname(const char *path)
> >
> > FILE *depfile; /* = NULL */
> > struct srcfile_state *current_srcfile; /* = NULL */
> > +static char *initial_path; /* = NULL */
> > +static int initial_pathlen; /* = 0 */
> > +static bool initial_cpp = true;
> >
> > -/* Detect infinite include recursion. */
> > -#define MAX_SRCFILE_DEPTH (100)
> > -static int srcfile_depth; /* = 0 */
> > +static void set_initial_path(char *fname)
> > +{
> > + int i, len = strlen(fname);
> >
> > + xasprintf(&initial_path, "%s", fname);
>
> Could just use an xstrdup() here.
>
> > + initial_pathlen = 0;
> > + for (i = 0; i != len; i++)
> > + if (initial_path[i] == '/')
> > + initial_pathlen++;
>
> What will happen if the user put's a double slash somewhere in one of
> the include paths?
>
> > +}
> > +
> > +static char *shorten_to_initial_path(char *fname)
> > +{
> > + char *p1, *p2, *prevslash1 = NULL;
> > + int slashes = 0;
> > +
> > + for (p1 = fname, p2 = initial_path; *p1 && *p2; p1++, p2++) {
> > + if (*p1 != *p2)
> > + break;
> > + if (*p1 == '/') {
> > + prevslash1 = p1;
> > + slashes++;
> > + }
> > + }
> > + p1 = prevslash1 + 1;
> > + if (prevslash1) {
> > + int diff = initial_pathlen - slashes, i, j;
> > + int restlen = strlen(fname) - (p1 - fname);
> > + char *res;
> > +
> > + res = xmalloc((3 * diff) + restlen + 1);
> > + for (i = 0, j = 0; i != diff; i++) {
> > + res[j++] = '.';
> > + res[j++] = '.';
> > + res[j++] = '/';
> > + }
> > + strcpy(res + j, p1);
> > + return res;
> > + }
> > + return fname;
> > +}
> >
> > /**
> > * Try to open a file in a given directory.
> > @@ -157,6 +200,9 @@ void srcfile_push(const char *fname)
> > srcfile->colno = 1;
> >
> > current_srcfile = srcfile;
> > +
> > + if (srcfile_depth == 1)
> > + set_initial_path(srcfile->name);
> > }
> >
> > bool srcfile_pop(void)
> > @@ -243,13 +289,10 @@ srcpos_copy(struct srcpos *pos)
> > pos_new = xmalloc(sizeof(struct srcpos));
> > memcpy(pos_new, pos, sizeof(struct srcpos));
> >
> > - 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;
> > - }
> > + /* allocate without free */
> > + srcfile_state = xmalloc(sizeof(struct srcfile_state));
> > + memcpy(srcfile_state, pos->file, sizeof(struct srcfile_state));
> > + pos_new->file = srcfile_state;
>
> Ah.. this hunk should be folded into the previous patch.
>
> >
> > return pos_new;
> > }
> > @@ -260,7 +303,7 @@ static bool same_pos(struct srcpos *p1, struct srcpos *p2)
> > p1->first_column == p2->first_column &&
> > p1->last_line == p2->last_line &&
> > p1->last_column == p2->last_column &&
> > - !strcmp(p1->file->name, p2->file->name));
> > + streq(p1->file->name, p2->file->name));
>
> As should this.
>
> > }
> >
> > struct srcpos *srcpos_extend(struct srcpos *pos, struct srcpos *newtail)
> > @@ -314,6 +357,60 @@ srcpos_string(struct srcpos *pos)
> > return pos_str;
> > }
> >
> > +static char *
> > +srcpos_string_comment(struct srcpos *pos, bool first_line, bool full)
> > +{
> > + char *pos_str, *fname, *first, *rest;
> > +
> > + if (!pos) {
> > + if (full) {
> > + xasprintf(&pos_str, "<no-file>:<no-line>");
> > + return pos_str;
> > + } else {
> > + return NULL;
> > + }
> > + }
> > +
> > + if (!pos->file)
> > + fname = "<no-file>";
> > + else if (!pos->file->name)
> > + fname = "<no-filename>";
> > + else if (full)
> > + fname = pos->file->name;
> > + else
> > + fname = shorten_to_initial_path(pos->file->name);
> > +
> > + if (full)
> > + xasprintf(&first, "%s:%d:%d-%d:%d", fname,
> > + pos->first_line, pos->first_column,
> > + pos->last_line, pos->last_column);
> > + else
> > + xasprintf(&first, "%s:%d", fname,
> > + first_line ? pos->first_line : pos->last_line);
> > +
> > + if (pos->next != NULL) {
> > + rest = srcpos_string_comment(pos->next, first_line, full);
> > + xasprintf(&pos_str, "%s, %s", first, rest);
> > + free(first);
> > + free(rest);
> > + } else {
> > + pos_str = first;
> > + }
> > +
> > + return pos_str;
> > +}
> > +
> > +char *srcpos_string_first(struct srcpos *pos, bool full)
> > +{
> > + return srcpos_string_comment(pos, true, full);
> > +}
> > +
> > +char *srcpos_string_last(struct srcpos *pos, bool full)
> > +{
> > + return srcpos_string_comment(pos, false, full);
> > +}
> > +
> > +
> > void srcpos_verror(struct srcpos *pos, const char *prefix,
> > const char *fmt, va_list va)
> > {
> > @@ -342,4 +439,9 @@ void srcpos_set_line(char *f, int l)
> > {
> > current_srcfile->name = f;
> > current_srcfile->lineno = l;
> > +
> > + if (initial_cpp) {
> > + initial_cpp = false;
> > + set_initial_path(f);
> > + }
> > }
> > diff --git a/srcpos.h b/srcpos.h
> > index d88e7cb..353c040 100644
> > --- a/srcpos.h
> > +++ b/srcpos.h
> > @@ -109,6 +109,8 @@ extern struct srcpos *srcpos_copy(struct srcpos *pos);
> > extern struct srcpos *srcpos_extend(struct srcpos *new_srcpos,
> > struct srcpos *old_srcpos);
> > extern char *srcpos_string(struct srcpos *pos);
> > +extern char *srcpos_string_first(struct srcpos *pos, bool full);
> > +extern char *srcpos_string_last(struct srcpos *pos, bool full);
> >
> > extern void PRINTF(3, 0) srcpos_verror(struct srcpos *pos, const char *prefix,
> > const char *fmt, va_list va);
> > diff --git a/treesource.c b/treesource.c
> > index 2461a3d..f454ba4 100644
> > --- a/treesource.c
> > +++ b/treesource.c
> > @@ -199,10 +199,20 @@ static void write_propval(FILE *f, struct property *prop)
> > struct marker *m = prop->val.markers;
> > int nnotstring = 0, nnul = 0;
> > int nnotstringlbl = 0, nnotcelllbl = 0;
> > + char *srcstr;
> > int i;
> >
> > if (len == 0) {
> > - fprintf(f, ";\n");
> > + fprintf(f, ";");
> > + if (annotate) {
> > + srcstr = srcpos_string_first(prop->srcpos,
> > + annotate_full);
> > + if (srcstr) {
> > + fprintf(f, " /* %s */", srcstr);
> > + free(srcstr);
> > + }
> > + }
> > + fprintf(f, "\n");
> > return;
> > }
> >
> > @@ -230,7 +240,15 @@ static void write_propval(FILE *f, struct property *prop)
> > write_propval_bytes(f, prop->val);
> > }
> >
> > - fprintf(f, ";\n");
> > + fprintf(f, ";");
> > + if (annotate) {
> > + srcstr = srcpos_string_first(prop->srcpos, annotate_full);
> > + if (srcstr) {
> > + fprintf(f, " /* %s */", srcstr);
> > + free(srcstr);
> > + }
> > + }
> > + fprintf(f, "\n");
> > }
> >
> > static void write_tree_source_node(FILE *f, struct node *tree, int level)
> > @@ -238,14 +256,24 @@ static void write_tree_source_node(FILE *f, struct node *tree, int level)
> > struct property *prop;
> > struct node *child;
> > struct label *l;
> > + char *srcstr;
> >
> > write_prefix(f, level);
> > for_each_label(tree->labels, l)
> > fprintf(f, "%s: ", l->label);
> > if (tree->name && (*tree->name))
> > - fprintf(f, "%s {\n", tree->name);
> > + fprintf(f, "%s {", tree->name);
> > else
> > - fprintf(f, "/ {\n");
> > + fprintf(f, "/ {");
> > +
> > + if (annotate) {
> > + srcstr = srcpos_string_first(tree->srcpos, annotate_full);
> > + if (srcstr) {
> > + fprintf(f, " /* %s */", srcstr);
> > + free(srcstr);
> > + }
> > + }
> > + fprintf(f, "\n");
> >
> > for_each_property(tree, prop) {
> > write_prefix(f, level+1);
> > @@ -259,10 +287,17 @@ static void write_tree_source_node(FILE *f, struct node *tree, int level)
> > write_tree_source_node(f, child, level+1);
> > }
> > write_prefix(f, level);
> > - fprintf(f, "};\n");
> > + fprintf(f, "};");
> > + if (annotate) {
> > + srcstr = srcpos_string_last(tree->srcpos, annotate_full);
> > + if (srcstr) {
> > + fprintf(f, " /* %s */", srcstr);
> > + free(srcstr);
> > + }
> > + }
> > + fprintf(f, "\n");
> > }
> >
> > -
> > void dt_to_source(FILE *f, struct dt_info *dti)
> > {
> > struct reserve_info *re;
>
> --
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
