Re: [PATCH 2/3] Look for include files in the directory of the including file.
So, like, the other day David Gibson mumbled: > > The doubly-linked list is intended to make it easier to construct search > > path lists one-at-a-time from arguments in the proper order, without > > needing to reverse the list at the end. > > We've already got that problem with a bunch of the lists we create > during parsing (we have several ugly add-to-end-of-singly-linked-list > functions). Going to doubly-linked lists might not be a bad idea, but > we should do it across the board, probably using the kernel's list.h > or something like it. I would be happy to accept a kernel's-list.h-based refactoring for doubly linked lists after the imminent 1.1 release! jdl ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/3] Look for include files in the directory of the including file.
On Sun, Jan 06, 2008 at 04:52:52PM -0600, Scott Wood wrote:
> On Fri, Jan 04, 2008 at 03:27:39PM +1100, David Gibson wrote:
> > > + newfile = dtc_open_file(filename, searchptr);
> > > + if (!newfile) {
> > > + yyerrorf("Couldn't open \"%s\": %s",
> > > + filename, strerror(errno));
> > > + exit(1);
> >
> > Use die() here, that's what it's for.
>
> die() doesn't print file and line information.
>
> > > + while (search) {
> > > + if (dtc_open_one(file, search->dir, fname))
> > > + return file;
> >
> > Don't we need a different case here somewhere for if someone specifies
> > an include file as an absolute path? Have I missed something?
>
> Yeah, I forgot about that, and sent another patch to fix it when I
> noticed (jdl had already pulled, so I didn't send an amended patch).
>
> > [snip]
> > > +struct search_path {
> > > + const char *dir; /* NULL for current directory */
> > > + struct search_path *prev, *next;
> > > +};
> >
> > I wouldn't suggest a doubly linked list here. Or at least not without
> > converting our many existing singly linked lists at the same time.
>
> The doubly-linked list is intended to make it easier to construct search
> path lists one-at-a-time from arguments in the proper order, without
> needing to reverse the list at the end.
We've already got that problem with a bunch of the lists we create
during parsing (we have several ugly add-to-end-of-singly-linked-list
functions). Going to doubly-linked lists might not be a bad idea, but
we should do it across the board, probably using the kernel's list.h
or something like it.
--
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
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/3] Look for include files in the directory of the including file.
On Fri, Jan 04, 2008 at 03:27:39PM +1100, David Gibson wrote:
> > + newfile = dtc_open_file(filename, searchptr);
> > + if (!newfile) {
> > + yyerrorf("Couldn't open \"%s\": %s",
> > +filename, strerror(errno));
> > + exit(1);
>
> Use die() here, that's what it's for.
die() doesn't print file and line information.
> > + while (search) {
> > + if (dtc_open_one(file, search->dir, fname))
> > + return file;
>
> Don't we need a different case here somewhere for if someone specifies
> an include file as an absolute path? Have I missed something?
Yeah, I forgot about that, and sent another patch to fix it when I
noticed (jdl had already pulled, so I didn't send an amended patch).
> [snip]
> > +struct search_path {
> > + const char *dir; /* NULL for current directory */
> > + struct search_path *prev, *next;
> > +};
>
> I wouldn't suggest a doubly linked list here. Or at least not without
> converting our many existing singly linked lists at the same time.
The doubly-linked list is intended to make it easier to construct search
path lists one-at-a-time from arguments in the proper order, without
needing to reverse the list at the end.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/3] Look for include files in the directory of the including file.
On Thu, Jan 03, 2008 at 05:43:31PM -0600, Scott Wood wrote:
> Looking in the diretory dtc is invoked from is not very useful behavior.
>
> As part of the code reorganization to implement this, I removed the
> uniquifying of name storage -- it seemed a rather dubious optimization
> given likely usage, and some aspects of it would have been mildly awkward
> to integrate with the new code.
>
> Signed-off-by: Scott Wood <[EMAIL PROTECTED]>
Would have been kind of nice to see the two parts as separate
patches. But no big deal, again, I'd really like to see this in for
1.1.
Acked-by: David Gibson <[EMAIL PROTECTED]>
A few comments nonetheless:
[snip]
> @@ -260,7 +259,19 @@ int push_input_file(const char *filename)
> return 0;
> }
>
> - f = dtc_open_file(filename);
> + if (srcpos_file) {
> + search.dir = srcpos_file->dir;
> + search.next = NULL;
> + search.prev = NULL;
> + searchptr = &search;
> + }
> +
> + newfile = dtc_open_file(filename, searchptr);
> + if (!newfile) {
> + yyerrorf("Couldn't open \"%s\": %s",
> + filename, strerror(errno));
> + exit(1);
Use die() here, that's what it's for.
> + }
>
> incl_file = malloc(sizeof(struct incl_file));
> if (!incl_file) {
And we should be using xmalloc() here (not your change, I realise).
[snip]
> -FILE *dtc_open_file(const char *fname)
> +static int dtc_open_one(struct dtc_file *file,
> +const char *search,
> +const char *fname)
> {
> - FILE *f;
> -
> - if (lookup_file_name(fname, 1) < 0)
> - die("Too many files opened\n");
> -
> - if (streq(fname, "-"))
> - f = stdin;
> - else
> - f = fopen(fname, "r");
> + char *fullname;
> +
> + if (search) {
> + fullname = malloc(strlen(search) + strlen(fname) + 2);
> + if (!fullname)
> + die("Out of memory\n");
xmalloc() again (your fault, this time).
> + strcpy(fullname, search);
> + strcat(fullname, "/");
> + strcat(fullname, fname);
> + } else {
> + fullname = strdup(fname);
> + }
>
> - if (! f)
> - die("Couldn't open \"%s\": %s\n", fname, strerror(errno));
> + file->file = fopen(fullname, "r");
> + if (!file->file) {
> + free(fullname);
> + return 0;
> + }
>
> - return f;
> + file->name = fullname;
> + return 1;
> }
>
>
> -
> -/*
> - * Locate and optionally add filename fname in the file_names[] array.
> - *
> - * If the filename is currently not in the array and the boolean
> - * add_it is non-zero, an attempt to add the filename will be made.
> - *
> - * Returns;
> - *Index [0..MAX_N_FILE_NAMES) where the filename is kept
> - *-1 if the name can not be recorded
> - */
> -
> -int lookup_file_name(const char *fname, int add_it)
> +struct dtc_file *dtc_open_file(const char *fname,
> + const struct search_path *search)
> {
> - int i;
> -
> - for (i = 0; i < n_file_names; i++) {
> - if (strcmp(file_names[i], fname) == 0)
> - return i;
> + static const struct search_path default_search = { NULL, NULL, NULL };
> +
> + struct dtc_file *file;
> + const char *slash;
> +
> + file = malloc(sizeof(struct dtc_file));
> + if (!file)
> + die("Out of memory\n");
xmalloc() again.
> + slash = strrchr(fname, '/');
> + if (slash) {
> + char *dir = malloc(slash - fname + 1);
> + if (!dir)
> + die("Out of memory\n");
And again.
> + memcpy(dir, fname, slash - fname);
> + dir[slash - fname] = 0;
> + file->dir = dir;
> + } else {
> + file->dir = NULL;
> }
>
> - if (add_it) {
> - if (n_file_names < MAX_N_FILE_NAMES) {
> - file_names[n_file_names] = strdup(fname);
> - return n_file_names++;
> - }
> + if (streq(fname, "-")) {
> + file->name = "stdin";
> + file->file = stdin;
> + return file;
> }
>
> - return -1;
> -}
> + if (!search)
> + search = &default_search;
>
> + while (search) {
> + if (dtc_open_one(file, search->dir, fname))
> + return file;
Don't we need a different case here somewhere for if someone specifies
an include file as an absolute path? Have I missed something?
[snip]
> +struct search_path {
> + const char *dir; /* NULL for current directory */
> + struct search_path *prev, *next;
> +};
I wouldn't suggest a doubly linked list here. Or at least not without
converting our many existing singly linked lists at the same time.
--
David Gibson| I'll have my music baroque,
[PATCH 2/3] Look for include files in the directory of the including file.
Looking in the diretory dtc is invoked from is not very useful behavior.
As part of the code reorganization to implement this, I removed the
uniquifying of name storage -- it seemed a rather dubious optimization
given likely usage, and some aspects of it would have been mildly awkward
to integrate with the new code.
Signed-off-by: Scott Wood <[EMAIL PROTECTED]>
---
dtc-lexer.l | 64 +
dtc-parser.y |2 +-
dtc.c| 14 --
srcpos.c | 126 +++---
srcpos.h | 28 +
5 files changed, 134 insertions(+), 100 deletions(-)
diff --git a/dtc-lexer.l b/dtc-lexer.l
index c811b22..bfb996e 100644
--- a/dtc-lexer.l
+++ b/dtc-lexer.l
@@ -74,7 +74,7 @@ static int dts_version; /* = 0 */
}
<*>\"([^\\"]|\\.)*\" {
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
DPRINT("String: %s\n", yytext);
yylval.data = data_copy_escape_string(yytext+1,
@@ -84,7 +84,7 @@ static int dts_version; /* = 0 */
}
<*>"/dts-v1/" {
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
DPRINT("Keyword: /dts-v1/\n");
dts_version = 1;
@@ -93,7 +93,7 @@ static int dts_version; /* = 0 */
}
<*>"/memreserve/" {
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
DPRINT("Keyword: /memreserve/\n");
BEGIN_DEFAULT();
@@ -101,7 +101,7 @@ static int dts_version; /* = 0 */
}
<*>{LABEL}:{
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
DPRINT("Label: %s\n", yytext);
yylval.labelref = strdup(yytext);
@@ -110,7 +110,7 @@ static int dts_version; /* = 0 */
}
[bodh]# {
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
if (*yytext == 'b')
yylval.cbase = 2;
@@ -125,7 +125,7 @@ static int dts_version; /* = 0 */
}
[0-9a-fA-F]+ {
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
yylval.literal = strdup(yytext);
DPRINT("Literal: '%s'\n", yylval.literal);
@@ -133,7 +133,7 @@ static int dts_version; /* = 0 */
}
[0-9]+|0[xX][0-9a-fA-F]+ {
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
yylval.literal = strdup(yytext);
DPRINT("Literal: '%s'\n", yylval.literal);
@@ -141,7 +141,7 @@ static int dts_version; /* = 0 */
}
\&{LABEL} { /* label reference */
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
DPRINT("Ref: %s\n", yytext+1);
yylval.labelref = strdup(yytext+1);
@@ -149,7 +149,7 @@ static int dts_version; /* = 0 */
}
"&{/"{PATHCHAR}+\} { /* new-style path reference */
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
yytext[yyleng-1] = '\0';
DPRINT("Ref: %s\n", yytext+2);
@@ -158,7 +158,7 @@ static int dts_version; /* = 0 */
}
"&/"{PATHCHAR}+ { /* old-style path reference */
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
DPRINT("Ref: %s\n", yytext+1);
yylval.labelref = strdup(yytext+1);
@@ -166,7 +166,7 @@ static int dts_version; /* = 0 */
}
[0-9a-fA-F]{2} {
- yylloc.filenum = srcpos_filenum;
+ yylloc.file = srcpos_file;
yylloc.first_line = yylineno;
yylval.byte = strtol(yytext, NULL, 16);
DPRINT("Byte: %02x\n", (int)yylval.byte);
@@ -174,7 +174,7 @@ static int dts_version; /* = 0 */
}
"]"{
