On Thu, Aug 16, 2018 at 12:00:42PM +0200, Julia Lawall wrote:
>
>
> 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).
>
> So is what is wanted only -T and --annotate?
Just those, with -T repeated to give the effect of --annotate-full.
> Or -T, --annotate and
> --annotate-full?
>
> thanks,
> 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
Attachment:
signature.asc
Description: PGP signature
