On Sunday 21 September 2008, Ned Ludd wrote: > I've been toying with an idea after seeing a patch which would abort() > when it detected host includes for cross compiles. I did not like the > idea of aborting as it prevented me from building pkgs that had the same > headers in $ROOT as /. I did like the fact there was a little QA I could > slip in there. After thinking about it a while. I said fsck the QA and > just fixed the problem. Here is what I came up with for gcc-4.2.4.. > > While in my testing I'm finding this works beautifully. However I'd like > some input from others on what they think of such an idea. > Good/Bad/Other? > > Basic goal detect ^/usr/include and rewrite it to $ROOT/usr/include if > using a cross compiler and ROOT is set.
add it to gcc already and be done. you nicely protect it with CROSS_COMPILE,
so it wont damage native users.
> +#ifdef CROSS_COMPILE
> +/* Rewrite the include paths for cross compiles */
> +char *cross_fixup_path(char *path);
> +char *cross_fixup_path(char *path) {
should be marked static right ?
> + char *name, *root, *ptr;
> + int len;
> +
> + root = getenv("ROOT");
> + if (root == NULL)
> + return name;
perhaps you mean to return path ? "name" is uninitialized here ...
> + if (strstr(path, "/usr/include") != path)
> + return path;
seems like a funky check and one that would throw false positives ... it's
valid for people to have /some/crazy/path/usr/include/ ...
make it an array of bad paths and anchor the search to the start:
/usr/local/include
/usr/include
/sw/include
also, are paths "normalized" by this point by common code ? in other words,
if someone does "-I////usr////include////", does gcc normailize it for you
already ? if not, should do so here ...
> + name = xstrdup(path);
umm, why ? you dont use the "name" storage at all ... in fact, you leak
it ...
> + len = strlen(root) + strlen(name) + 2;
> + ptr = (char *) xmalloc (len);
pointless cast
> + sprintf(ptr, "%s/%s", root, name);
doesnt gcc (via libiberty) already provide asprintf() ? that'd make this code
simpler ...
> + fprintf(stderr, _("Autofixing Invalid Cross Include Path: %s ->
> %s\n"), name, ptr);
i think gcc already has a set of warn-type functions which you should use
> + free(path);
> + path = ptr;
> + name = path;
> + return name;
why not just drop this "name" variable altogether ? just hit up "path"
directly
> +}
> +#endif
> +
> /* Add PATH to the include chain CHAIN. PATH must be malloc-ed and
> NUL-terminated. */
> void
> @@ -359,6 +385,11 @@
> p->construct = 0;
> p->user_supplied_p = user_supplied_p;
>
> +#ifdef CROSS_COMPILE
> + path = cross_fixup_path(path);
> + p->name = path;
> +#endif
> +
> add_cpp_dir_path (p, chain);
> }
seems kind of pointless to touch "path" here since ... i'm assuming it's
function local though (havent read the actual file)
-mike
signature.asc
Description: This is a digitally signed message part.
