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, 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
