From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Gibson Subject: Re: [PATCH 3/3 v2] annotations: add the annotation functionality Date: Wed, 17 Jan 2018 12:13:22 +1100 Message-ID: <20180117011322.GW30352@umbus.fritz.box> References: <1516040650-9848-1-git-send-email-Julia.Lawall@lip6.fr> <1516040650-9848-4-git-send-email-Julia.Lawall@lip6.fr> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="cxfMsoqvp1jUizWj" Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1516151609; bh=oPwU4wr5Nt+oL7jPfyQgDkeRuj5f8UIiF45pESGxxgs=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=UYuS0FrO5K8vxyfRm+RdFiCmt9OM25B2+KXNnFivTg8dfMWdOrETwSWcMLRHE1eB0 ty5vox1+ILtVPckVMA8bRpcVi/kY/GZ6PljyM5j5vNhaMhPn+c5Z8+SlJHyWjFnzfe 2YzqcnPXyziUxctuPb78BY89QxW9LPZ/1VV2UjdE= Content-Disposition: inline In-Reply-To: <1516040650-9848-4-git-send-email-Julia.Lawall-L2FTfq7BK8M@public.gmane.org> Sender: devicetree-compiler-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: To: Julia Lawall Cc: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Frank Rowand --cxfMsoqvp1jUizWj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jan 15, 2018 at 07:24:10PM +0100, Julia Lawall wrote: > This commit provides two new command-line options: >=20 > --annotate (abbreviated -T) > --annotate-full (abbreviated -F) What's the rationale for having two different versions of the annotations? >=20 > --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. >=20 > --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 :. >=20 > 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. >=20 > 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. >=20 > Note that the column numbers in the --annotate-full case may be of > limited use when cpp is first used, because the columns are those in the > output generated by cpp, which are not the same as the ones in the > source code. It may be that cpp simply replaces tabs by spaces. >=20 > Various changes are sprinkled throughout treesource.c to print the > annotations. >=20 > Signed-off-by: Julia Lawall > Signed-off-by: Frank Rowand > --- >=20 > v2: Squash all of the annotation support into a single patch, enable > printing a list of positions. >=20 > dtc.c | 17 ++++++++- > dtc.h | 2 ++ > srcpos.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++-- > srcpos.h | 2 ++ > treesource.c | 47 +++++++++++++++++++++---- > 5 files changed, 169 insertions(+), 10 deletions(-) >=20 > diff --git a/dtc.c b/dtc.c > index c36994e..0153b1a 100644 > --- a/dtc.c > +++ b/dtc.c > @@ -35,6 +35,8 @@ int phandle_format =3D PHANDLE_EPAPR; /* Use linux,phan= dle 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; /* =3Dfalse, annotate .dts with input source location */ > +bool annotate_full; /* =3Dfalse, annotate .dts with full input source lo= cation */ > =20 > static int is_power_of_2(int x) > { > @@ -60,7 +62,7 @@ static void fill_fullpaths(struct node *tree, const cha= r *prefix) > =20 > /* Usage related data. */ > static const char usage_synopsis[] =3D "dtc [options] "; > -static const char usage_short_opts[] =3D "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E= :@Ahv"; > +static const char usage_short_opts[] =3D "qI:O:o:V:d:R:S:p:a:fb:i:H:sW:E= :@ATFhv"; > static struct option const usage_long_opts[] =3D { > {"quiet", no_argument, NULL, 'q'}, > {"in-format", a_argument, NULL, 'I'}, > @@ -81,6 +83,8 @@ static struct option const usage_long_opts[] =3D { > {"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[] =3D { > "\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 c= olumn", > "\n\tPrint this help and exit", > "\n\tPrint version and exit", > NULL, > @@ -259,6 +265,11 @@ int main(int argc, char *argv[]) > case 'A': > auto_label_aliases =3D 1; > break; > + case 'F': > + annotate_full =3D true; Uncommented fallthrough. Please don't do that. > + case 'T': > + annotate =3D true; > + break; > =20 > case 'h': > usage(NULL); > @@ -297,6 +308,10 @@ int main(int argc, char *argv[]) > outform =3D "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 =3D dt_from_source(arg); > else if (streq(inform, "fs")) > diff --git a/dtc.h b/dtc.h > index 3a85058..0b8141b 100644 > --- a/dtc.h > +++ b/dtc.h > @@ -58,6 +58,8 @@ extern int phandle_format; /* Use linux,phandle or phan= dle 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 locat= ion */ > =20 > #define PHANDLE_LEGACY 0x1 > #define PHANDLE_EPAPR 0x2 > diff --git a/srcpos.c b/srcpos.c > index 2ed794b..04e78ba 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; > =20 > +/* Detect infinite include recursion. */ > +#define MAX_SRCFILE_DEPTH (100) > +static int srcfile_depth; /* =3D 0 */ > =20 > static char *get_dirname(const char *path) > { > @@ -51,11 +54,51 @@ static char *get_dirname(const char *path) > =20 > FILE *depfile; /* =3D NULL */ > struct srcfile_state *current_srcfile; /* =3D NULL */ > +static char *initial_path; /* =3D NULL */ > +static int initial_pathlen; /* =3D 0 */ > +static bool initial_cpp =3D true; > =20 > -/* Detect infinite include recursion. */ > -#define MAX_SRCFILE_DEPTH (100) > -static int srcfile_depth; /* =3D 0 */ > +static void set_initial_path(char *fname) > +{ > + int i, len =3D strlen(fname); > + > + xasprintf(&initial_path, "%s", fname); > + initial_pathlen =3D 0; > + for (i =3D 0; i !=3D len; i++) > + if (initial_path[i] =3D=3D '/') > + initial_pathlen++; > +} > =20 > +static char *shorten_to_initial_path(char *fname) > +{ > + char *p1, *p2, *prevslash1 =3D NULL; > + int slashes =3D 0; > + > + for (p1 =3D fname, p2 =3D initial_path; *p1 && *p2; p1++, p2++) { > + if (*p1 !=3D *p2) > + break; > + if (*p1 =3D=3D '/') { > + prevslash1 =3D p1; > + slashes++; > + } > + } > + p1 =3D prevslash1 + 1; > + if (prevslash1) { > + int diff =3D initial_pathlen - slashes, i, j; > + int restlen =3D strlen(fname) - (p1 - fname); > + char *res; > + > + res =3D xmalloc((3 * diff) + restlen + 1); > + for (i =3D 0, j =3D 0; i !=3D diff; i++) { > + res[j++] =3D '.'; > + res[j++] =3D '.'; > + res[j++] =3D '/'; > + } > + strcpy(res + j, p1); > + return res; > + } > + return fname; > +} > =20 > /** > * Try to open a file in a given directory. > @@ -157,6 +200,9 @@ void srcfile_push(const char *fname) > srcfile->colno =3D 0; > =20 > current_srcfile =3D srcfile; > + > + if (srcfile_depth =3D=3D 1) > + set_initial_path(srcfile->name); > } > =20 > bool srcfile_pop(void) > @@ -351,6 +397,60 @@ srcpos_string(struct srcpos *pos) > return pos_str; > } > =20 > +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, ":"); > + return pos_str; > + } else { > + return NULL; > + } > + } > + > + if (!pos->file) > + fname =3D ""; > + else if (!pos->file->name) > + fname =3D ""; > + else if (full) > + fname =3D pos->file->name; > + else > + fname =3D 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 !=3D NULL) { > + rest =3D srcpos_string_comment(pos->next, first_line, full); > + xasprintf(&pos_str, "%s, %s", first, rest); > + free(first); > + free(rest); > + } else { > + pos_str =3D 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) > { > @@ -379,4 +479,9 @@ void srcpos_set_line(char *f, int l) > { > current_srcfile->name =3D f; > current_srcfile->lineno =3D l; > + > + if (initial_cpp) { > + initial_cpp =3D false; > + set_initial_path(f); > + } > } > diff --git a/srcpos.h b/srcpos.h > index c22d6ba..a538c4e 100644 > --- a/srcpos.h > +++ b/srcpos.h > @@ -111,6 +111,8 @@ extern struct srcpos *srcpos_combine(struct srcpos *l= eft_srcpos, > 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); > =20 > extern void PRINTF(3, 0) srcpos_verror(struct srcpos *pos, const char *p= refix, > 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 =3D prop->val.markers; > int nnotstring =3D 0, nnul =3D 0; > int nnotstringlbl =3D 0, nnotcelllbl =3D 0; > + char *srcstr; > int i; > =20 > if (len =3D=3D 0) { > - fprintf(f, ";\n"); > + fprintf(f, ";"); > + if (annotate) { > + srcstr =3D srcpos_string_first(prop->srcpos, > + annotate_full); > + if (srcstr) { > + fprintf(f, " /* %s */", srcstr); > + free(srcstr); > + } > + } > + fprintf(f, "\n"); > return; > } > =20 > @@ -230,7 +240,15 @@ static void write_propval(FILE *f, struct property *= prop) > write_propval_bytes(f, prop->val); > } > =20 > - fprintf(f, ";\n"); > + fprintf(f, ";"); > + if (annotate) { > + srcstr =3D srcpos_string_first(prop->srcpos, annotate_full); > + if (srcstr) { > + fprintf(f, " /* %s */", srcstr); > + free(srcstr); > + } > + } > + fprintf(f, "\n"); > } > =20 > 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; > =20 > 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 =3D srcpos_string_first(tree->srcpos, annotate_full); > + if (srcstr) { > + fprintf(f, " /* %s */", srcstr); > + free(srcstr); > + } > + } > + fprintf(f, "\n"); > =20 > 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 =3D srcpos_string_last(tree->srcpos, annotate_full); > + if (srcstr) { > + fprintf(f, " /* %s */", srcstr); > + free(srcstr); > + } > + } > + fprintf(f, "\n"); > } > =20 > - > void dt_to_source(FILE *f, struct dt_info *dti) > { > struct reserve_info *re; --=20 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 --cxfMsoqvp1jUizWj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEdfRlhq5hpmzETofcbDjKyiDZs5IFAlpeozEACgkQbDjKyiDZ s5Kr+xAAw8wQVXFfbjr8e1bvVO9BrTebB1vdCYSkUJRpYzCZzwLfeby+z5wI6gB2 vELdKVMnB4mk2rlQ5G4vjDXQ7k2bHnFK8XuvCXdXPb/FjC9mSP5w9J9Yd+NPWmwN nvbO7h9deRyLYyhtUsPMUBDKuD0IkuHugfYR04CEW0Yy+u/Egwn5wwi47D6aBHlv KgbCZeXMWQ6PbMXchqhTYrGFWswwLsOVFXlnDk3wGH0Y49rIMsQd9/YvkwHhtulS 7c51HXVotr/CIrAgX+pgB1dk3ylvAVdwOP3hptsPNxrPs/6ocvA8hRMdfMU1Xvjl qeedz1lPzTAUcGd0nKzzui5odUIZZvrHeXcqg2SACOp7kzGsN0dVJM5hJvCoMaHC hcXyP4KGZU/kxHnIb2YxhxdIBFvkhq9rzCHkNK9mwSbHEfV1vajsWtam5Q1ZGn/J FyuQD+jRjzuhwfjwWXammbrmhhYY8ydp4F+P6P+84aq7dftspLnHS7SdgLuotoUL MEPwgCj8TPhgibuDLH7fCaMohDjnonPs5jtU0djOTeyPcWVIngWHYiPKvHa0Q1mF SqfM/iDUpwPYAHX5hu5ZznWTqPwTnvTyp1fihPh1emtw97F51VEeHrztpESFrgfZ ZcnMaSlbvsCZteCpI8b23GluBWE/QWKmdhvkSCxW7gzkomW3Sa8= =w8ij -----END PGP SIGNATURE----- --cxfMsoqvp1jUizWj--