| 1 |
On Sunday 21 September 2008, Ned Ludd wrote: |
| 2 |
> I've been toying with an idea after seeing a patch which would abort() |
| 3 |
> when it detected host includes for cross compiles. I did not like the |
| 4 |
> idea of aborting as it prevented me from building pkgs that had the same |
| 5 |
> headers in $ROOT as /. I did like the fact there was a little QA I could |
| 6 |
> slip in there. After thinking about it a while. I said fsck the QA and |
| 7 |
> just fixed the problem. Here is what I came up with for gcc-4.2.4.. |
| 8 |
> |
| 9 |
> While in my testing I'm finding this works beautifully. However I'd like |
| 10 |
> some input from others on what they think of such an idea. |
| 11 |
> Good/Bad/Other? |
| 12 |
> |
| 13 |
> Basic goal detect ^/usr/include and rewrite it to $ROOT/usr/include if |
| 14 |
> using a cross compiler and ROOT is set. |
| 15 |
|
| 16 |
add it to gcc already and be done. you nicely protect it with CROSS_COMPILE, |
| 17 |
so it wont damage native users. |
| 18 |
|
| 19 |
> +#ifdef CROSS_COMPILE |
| 20 |
> +/* Rewrite the include paths for cross compiles */ |
| 21 |
> +char *cross_fixup_path(char *path); |
| 22 |
> +char *cross_fixup_path(char *path) { |
| 23 |
|
| 24 |
should be marked static right ? |
| 25 |
|
| 26 |
> + char *name, *root, *ptr; |
| 27 |
> + int len; |
| 28 |
> + |
| 29 |
> + root = getenv("ROOT"); |
| 30 |
> + if (root == NULL) |
| 31 |
> + return name; |
| 32 |
|
| 33 |
perhaps you mean to return path ? "name" is uninitialized here ... |
| 34 |
|
| 35 |
> + if (strstr(path, "/usr/include") != path) |
| 36 |
> + return path; |
| 37 |
|
| 38 |
seems like a funky check and one that would throw false positives ... it's |
| 39 |
valid for people to have /some/crazy/path/usr/include/ ... |
| 40 |
|
| 41 |
make it an array of bad paths and anchor the search to the start: |
| 42 |
/usr/local/include |
| 43 |
/usr/include |
| 44 |
/sw/include |
| 45 |
|
| 46 |
also, are paths "normalized" by this point by common code ? in other words, |
| 47 |
if someone does "-I////usr////include////", does gcc normailize it for you |
| 48 |
already ? if not, should do so here ... |
| 49 |
|
| 50 |
> + name = xstrdup(path); |
| 51 |
|
| 52 |
umm, why ? you dont use the "name" storage at all ... in fact, you leak |
| 53 |
it ... |
| 54 |
|
| 55 |
> + len = strlen(root) + strlen(name) + 2; |
| 56 |
> + ptr = (char *) xmalloc (len); |
| 57 |
|
| 58 |
pointless cast |
| 59 |
|
| 60 |
> + sprintf(ptr, "%s/%s", root, name); |
| 61 |
|
| 62 |
doesnt gcc (via libiberty) already provide asprintf() ? that'd make this code |
| 63 |
simpler ... |
| 64 |
|
| 65 |
> + fprintf(stderr, _("Autofixing Invalid Cross Include Path: %s -> |
| 66 |
> %s\n"), name, ptr); |
| 67 |
|
| 68 |
i think gcc already has a set of warn-type functions which you should use |
| 69 |
|
| 70 |
> + free(path); |
| 71 |
> + path = ptr; |
| 72 |
> + name = path; |
| 73 |
> + return name; |
| 74 |
|
| 75 |
why not just drop this "name" variable altogether ? just hit up "path" |
| 76 |
directly |
| 77 |
|
| 78 |
> +} |
| 79 |
> +#endif |
| 80 |
> + |
| 81 |
> /* Add PATH to the include chain CHAIN. PATH must be malloc-ed and |
| 82 |
> NUL-terminated. */ |
| 83 |
> void |
| 84 |
> @@ -359,6 +385,11 @@ |
| 85 |
> p->construct = 0; |
| 86 |
> p->user_supplied_p = user_supplied_p; |
| 87 |
> |
| 88 |
> +#ifdef CROSS_COMPILE |
| 89 |
> + path = cross_fixup_path(path); |
| 90 |
> + p->name = path; |
| 91 |
> +#endif |
| 92 |
> + |
| 93 |
> add_cpp_dir_path (p, chain); |
| 94 |
> } |
| 95 |
|
| 96 |
seems kind of pointless to touch "path" here since ... i'm assuming it's |
| 97 |
function local though (havent read the actual file) |
| 98 |
-mike |
Archives