Hi Tom,
On Tue, Feb 18, 2020 at 11:29 AM Tom Rini <tr...@konsulko.com> wrote:
>
> The previous kbuild resync
Precisely, it was "kconfig resync", not "kbuild resync".
> of e91610da7c8a ("kconfig: re-sync with Linux
> 4.17-rc4") accidentally did not sync the fixdep program.
I do not think it was accidental.
Kconfig and fixdep are separate programs.
You do not necessarily need to sync them together.
> This commit
> brings fixdep in line with the rest of that previous resync.
>
> This includes all of the following Linux kernel commits:
> fbfa9be9904e kbuild: move include/config/ksym/* to include/ksym/*
> 5b8ad96d1a44 fixdep: remove some false CONFIG_ matches
> 14a596a7e6fd fixdep: remove stale references to uml-config.h
> ab9ce9feed36 fixdep: use existing helper to check modular CONFIG options
> 87b95a81357d fixdep: refactor parse_dep_file()
> 5d1ef76f5a22 fixdep: move global variables to local variables of main()
> ccfe78873c22 fixdep: remove unneeded memcpy() in parse_dep_file()
> 4003fd80cba9 fixdep: factor out common code for reading files
> 01b5cbe7012f fixdep: use malloc() and read() to load dep_file to buffer
> 41f92cffba19 fixdep: remove unnecessary <arpa/inet.h> inclusion
> 7c2ec43a2154 fixdep: exit with error code in error branches of
> do_config_file()
> 4e433fc4d1a9 fixdep: trivial: typo fix and correction
> dee81e988674 fixdep: faster CONFIG_ search
> c1a95fda2a40 kbuild: add fine grained build dependencies for exported symbols
> d8329e35cc08 fixdep: accept extra dependencies on stdin
> 4c835b57b8de fixdep: constify strrcmp arguments
>
> Of note is that when applying dee81e988674 above our logic in that area
> required some careful consideration to continue to apply.
I checked it. The resync looks correct.
> We specifically do not include:
> 638e69cf2230 fixdep: do not ignore kconfig.h
> as this leads to build problems for us resulting in problems like:
> dts/.dt.dtb.o.cmd:5: *** unterminated call to function 'wildcard': missing
> ')'. Stop.
This is due to config_enabled(CONFIG_VAL(option##_MODULE))
in include/linux/kconfig.h
This line was unneeded in the first place
since U-Boot does not support module,
but I just wanted to make it aligned with Linux somehow.
I sent two patches.
Please choose either depending your workflow.
[A] http://patchwork.ozlabs.org/patch/1239950/
(If you want to fix the issue before re-sync)
[B] http://patchwork.ozlabs.org/patch/1239952/
(If you want to fix the issue after re-sync)
Then, you can import
638e69cf2230 fixdep: do not ignore kconfig.h
Thanks.
>
> Cc: Masahiro Yamada <masahi...@kernel.org>'
> Signed-off-by: Tom Rini <tr...@konsulko.com>
> ---
> scripts/basic/fixdep.c | 352 ++++++++++++++++++-----------------------
> 1 file changed, 151 insertions(+), 201 deletions(-)
>
> diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
> index da7fb2cd4dde..8c21dd08d9f7 100644
> --- a/scripts/basic/fixdep.c
> +++ b/scripts/basic/fixdep.c
> @@ -25,7 +25,7 @@
> *
> * So we play the same trick that "mkdep" played before. We replace
> * the dependency on autoconf.h by a dependency on every config
> - * option which is mentioned in any of the listed prequisites.
> + * option which is mentioned in any of the listed prerequisites.
> *
> * kconfig populates a tree in include/config/ with an empty file
> * for each config symbol and when the configuration is updated
> @@ -34,7 +34,7 @@
> * the config symbols are rebuilt.
> *
> * So if the user changes his CONFIG_HIS_DRIVER option, only the objects
> - * which depend on "include/linux/config/his/driver.h" will be rebuilt,
> + * which depend on "include/config/his/driver.h" will be rebuilt,
> * so most likely only his driver ;-)
> *
> * The idea above dates, by the way, back to Michael E Chastain, AFAIK.
> @@ -75,15 +75,14 @@
> * and then basically copies the .<target>.d file to stdout, in the
> * process filtering out the dependency on autoconf.h and adding
> * dependencies on include/config/my/option.h for every
> - * CONFIG_MY_OPTION encountered in any of the prequisites.
> + * CONFIG_MY_OPTION encountered in any of the prerequisites.
> *
> * It will also filter out all the dependencies on *.ver. We need
> * to make sure that the generated version checksum are globally up
> * to date before even starting the recursive build, so it's too late
> * at this point anyway.
> *
> - * The algorithm to grep for "CONFIG_..." is bit unusual, but should
> - * be fast ;-) We don't even try to really parse the header files, but
> + * We don't even try to really parse the header files, but
> * merely grep, i.e. if CONFIG_FOO is mentioned in a comment, it will
> * be picked up as well. It's not a problem with respect to
> * correctness, since that can only give too many dependencies, thus
> @@ -94,49 +93,57 @@
> * (Note: it'd be easy to port over the complete mkdep state machine,
> * but I don't think the added complexity is worth it)
> */
> -/*
> - * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend
> onto
> - * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do
> not
> - * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as
> - * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h,
> - * through arch/um/include/uml-config.h; this fixdep "bug" makes sure that
> - * those files will have correct dependencies.
> - */
>
> #include <sys/types.h>
> #include <sys/stat.h>
> -#include <sys/mman.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <string.h>
> #include <stdlib.h>
> #include <stdio.h>
> -#include <limits.h>
> #include <ctype.h>
> -#include <arpa/inet.h>
> -
> -#define INT_CONF ntohl(0x434f4e46)
> -#define INT_ONFI ntohl(0x4f4e4649)
> -#define INT_NFIG ntohl(0x4e464947)
> -#define INT_FIG_ ntohl(0x4649475f)
>
> -char *target;
> -char *depfile;
> -char *cmdline;
> int is_spl_build = 0; /* hack for U-Boot */
>
> static void usage(void)
> {
> - fprintf(stderr, "Usage: fixdep <depfile> <target> <cmdline>\n");
> + fprintf(stderr, "Usage: fixdep [-e] <depfile> <target> <cmdline>\n");
> + fprintf(stderr, " -e insert extra dependencies given on stdin\n");
> exit(1);
> }
>
> /*
> - * Print out the commandline prefixed with cmd_<target filename> :=
> + * Print out a dependency path from a symbol name
> */
> -static void print_cmdline(void)
> +static void print_dep(const char *m, int slen, const char *dir)
> {
> - printf("cmd_%s := %s\n\n", target, cmdline);
> + int c, i;
> +
> + printf(" $(wildcard %s/", dir);
> + for (i = 0; i < slen; i++) {
> + c = m[i];
> + if (c == '_')
> + c = '/';
> + else
> + c = tolower(c);
> + putchar(c);
> + }
> + printf(".h) \\\n");
> +}
> +
> +static void do_extra_deps(void)
> +{
> + char buf[80];
> +
> + while (fgets(buf, sizeof(buf), stdin)) {
> + int len = strlen(buf);
> +
> + if (len < 2 || buf[len - 1] != '\n') {
> + fprintf(stderr, "fixdep: bad data on stdin\n");
> + exit(1);
> + }
> + print_dep(buf, len - 1, "include/ksym");
> + }
> }
>
> struct item {
> @@ -198,57 +205,44 @@ static void define_config(const char *name, int len,
> unsigned int hash)
> static void use_config(const char *m, int slen)
> {
> unsigned int hash = strhash(m, slen);
> - int c, i;
>
> if (is_defined_config(m, slen, hash))
> return;
>
> define_config(m, slen, hash);
> + print_dep(m, slen, "include/config");
> +}
>
> - printf(" $(wildcard include/config/");
> - for (i = 0; i < slen; i++) {
> - c = m[i];
> - if (c == '_')
> - c = '/';
> - else
> - c = tolower(c);
> - putchar(c);
> - }
> - printf(".h) \\\n");
> +/* test if s ends in sub */
> +static int str_ends_with(const char *s, int slen, const char *sub)
> +{
> + int sublen = strlen(sub);
> +
> + if (sublen > slen)
> + return 0;
> +
> + return !memcmp(s + slen - sublen, sub, sublen);
> }
>
> -static void parse_config_file(const char *map, size_t len)
> +static void parse_config_file(const char *p)
> {
> - const int *end = (const int *) (map + len);
> - /* start at +1, so that p can never be < map */
> - const int *m = (const int *) map + 1;
> - const char *p, *q;
> + const char *q, *r;
> + const char *start = p;
> char tmp_buf[256] = "SPL_"; /* hack for U-Boot */
>
> - for (; m < end; m++) {
> - if (*m == INT_CONF) { p = (char *) m ; goto conf; }
> - if (*m == INT_ONFI) { p = (char *) m-1; goto conf; }
> - if (*m == INT_NFIG) { p = (char *) m-2; goto conf; }
> - if (*m == INT_FIG_) { p = (char *) m-3; goto conf; }
> - continue;
> - conf:
> - if (p > map + len - 7)
> - continue;
> - if (memcmp(p, "CONFIG_", 7))
> + while ((p = strstr(p, "CONFIG_"))) {
> + if (p > start && (isalnum(p[-1]) || p[-1] == '_')) {
> + p += 7;
> continue;
> - p += 7;
> - for (q = p; q < map + len; q++) {
> - if (!(isalnum(*q) || *q == '_'))
> - goto found;
> }
> - continue;
> -
> - found:
> - if (!memcmp(q - 7, "_MODULE", 7))
> - q -= 7;
> - if (q - p < 0)
> - continue;
> -
> + p += 7;
> + q = p;
> + while (*q && (isalnum(*q) || *q == '_'))
> + q++;
> + if (str_ends_with(p, q - p, "_MODULE"))
> + r = q - 7;
> + else
> + r = q;
> /*
> * U-Boot also handles
> * CONFIG_IS_ENABLED(...)
> @@ -261,69 +255,62 @@ static void parse_config_file(const char *map, size_t
> len)
> (q - p == 9 && !memcmp(p, "IS_MODULE(", 10)) ||
> (q - p == 3 && !memcmp(p, "VAL(", 4))) {
> p = q + 1;
> - for (q = p; q < map + len; q++)
> - if (*q == ')')
> - goto found2;
> - continue;
> -
> - found2:
> - if (is_spl_build) {
> - memcpy(tmp_buf + 4, p, q - p);
> - q = tmp_buf + 4 + (q - p);
> + while (*q && *q != ')')
> + q++;
> + r = q;
> + if (r > p && is_spl_build) {
> + memcpy(tmp_buf + 4, p, r - p);
> + r = tmp_buf + 4 + (r - p);
> p = tmp_buf;
> }
> }
> /* end U-Boot hack */
>
> - use_config(p, q - p);
> + if (r > p)
> + use_config(p, r - p);
> + p = q;
> }
> }
>
> -/* test is s ends in sub */
> -static int strrcmp(char *s, char *sub)
> -{
> - int slen = strlen(s);
> - int sublen = strlen(sub);
> -
> - if (sublen > slen)
> - return 1;
> -
> - return memcmp(s + slen - sublen, sub, sublen);
> -}
> -
> -static void do_config_file(const char *filename)
> +static void *read_file(const char *filename)
> {
> struct stat st;
> int fd;
> - void *map;
> + char *buf;
>
> fd = open(filename, O_RDONLY);
> if (fd < 0) {
> - fprintf(stderr, "fixdep: error opening config file: ");
> + fprintf(stderr, "fixdep: error opening file: ");
> perror(filename);
> exit(2);
> }
> if (fstat(fd, &st) < 0) {
> - fprintf(stderr, "fixdep: error fstat'ing config file: ");
> + fprintf(stderr, "fixdep: error fstat'ing file: ");
> perror(filename);
> exit(2);
> }
> - if (st.st_size == 0) {
> - close(fd);
> - return;
> + buf = malloc(st.st_size + 1);
> + if (!buf) {
> + perror("fixdep: malloc");
> + exit(2);
> }
> - map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
> - if ((long) map == -1) {
> - perror("fixdep: mmap");
> - close(fd);
> - return;
> + if (read(fd, buf, st.st_size) != st.st_size) {
> + perror("fixdep: read");
> + exit(2);
> }
> + buf[st.st_size] = '\0';
> + close(fd);
>
> - parse_config_file(map, st.st_size);
> -
> - munmap(map, st.st_size);
> + return buf;
> +}
>
> - close(fd);
> +/* Ignore certain dependencies */
> +static int is_ignored_file(const char *s, int len)
> +{
> + return str_ends_with(s, len, "include/generated/autoconf.h") ||
> + str_ends_with(s, len, "include/generated/autoksyms.h") ||
> + str_ends_with(s, len, "include/linux/kconfig.h") ||
> + str_ends_with(s, len, ".ver");
> }
>
> /*
> @@ -331,70 +318,70 @@ static void do_config_file(const char *filename)
> * assignments are parsed not only by make, but also by the rather simple
> * parser in scripts/mod/sumversion.c.
> */
> -static void parse_dep_file(void *map, size_t len)
> +static void parse_dep_file(char *m, const char *target, int
> insert_extra_deps)
> {
> - char *m = map;
> - char *end = m + len;
> char *p;
> - char s[PATH_MAX];
> - int is_target;
> + int is_last, is_target;
> int saw_any_target = 0;
> int is_first_dep = 0;
> + void *buf;
>
> - while (m < end) {
> + while (1) {
> /* Skip any "white space" */
> - while (m < end && (*m == ' ' || *m == '\\' || *m == '\n'))
> + while (*m == ' ' || *m == '\\' || *m == '\n')
> m++;
> +
> + if (!*m)
> + break;
> +
> /* Find next "white space" */
> p = m;
> - while (p < end && *p != ' ' && *p != '\\' && *p != '\n')
> + while (*p && *p != ' ' && *p != '\\' && *p != '\n')
> p++;
> + is_last = (*p == '\0');
> /* Is the token we found a target name? */
> is_target = (*(p-1) == ':');
> /* Don't write any target names into the dependency file */
> if (is_target) {
> /* The /next/ file is the first dependency */
> is_first_dep = 1;
> - } else {
> - /* Save this token/filename */
> - memcpy(s, m, p-m);
> - s[p - m] = 0;
> -
> - /* Ignore certain dependencies */
> - if (strrcmp(s, "include/generated/autoconf.h") &&
> - strrcmp(s, "arch/um/include/uml-config.h") &&
> - strrcmp(s, "include/linux/kconfig.h") &&
> - strrcmp(s, ".ver")) {
> + } else if (!is_ignored_file(m, p - m)) {
> + *p = '\0';
> +
> + /*
> + * Do not list the source file as dependency, so that
> + * kbuild is not confused if a .c file is rewritten
> + * into .S or vice versa. Storing it in source_* is
> + * needed for modpost to compute srcversions.
> + */
> + if (is_first_dep) {
> /*
> - * Do not list the source file as dependency,
> - * so that kbuild is not confused if a .c file
> - * is rewritten into .S or vice versa. Storing
> - * it in source_* is needed for modpost to
> - * compute srcversions.
> + * If processing the concatenation of multiple
> + * dependency files, only process the first
> + * target name, which will be the original
> + * source name, and ignore any other target
> + * names, which will be intermediate temporary
> + * files.
> */
> - if (is_first_dep) {
> - /*
> - * If processing the concatenation of
> - * multiple dependency files, only
> - * process the first target name,
> which
> - * will be the original source name,
> - * and ignore any other target names,
> - * which will be intermediate
> temporary
> - * files.
> - */
> - if (!saw_any_target) {
> - saw_any_target = 1;
> - printf("source_%s := %s\n\n",
> - target, s);
> - printf("deps_%s := \\\n",
> - target);
> - }
> - is_first_dep = 0;
> - } else
> - printf(" %s \\\n", s);
> - do_config_file(s);
> + if (!saw_any_target) {
> + saw_any_target = 1;
> + printf("source_%s := %s\n\n",
> + target, m);
> + printf("deps_%s := \\\n", target);
> + }
> + is_first_dep = 0;
> + } else {
> + printf(" %s \\\n", m);
> }
> +
> + buf = read_file(m);
> + parse_config_file(buf);
> + free(buf);
> }
> +
> + if (is_last)
> + break;
> +
> /*
> * Start searching for next token immediately after the first
> * "whitespace" character that follows this token.
> @@ -407,63 +394,23 @@ static void parse_dep_file(void *map, size_t len)
> exit(1);
> }
>
> + if (insert_extra_deps)
> + do_extra_deps();
> +
> printf("\n%s: $(deps_%s)\n\n", target, target);
> printf("$(deps_%s):\n", target);
> }
>
> -static void print_deps(void)
> -{
> - struct stat st;
> - int fd;
> - void *map;
> -
> - fd = open(depfile, O_RDONLY);
> - if (fd < 0) {
> - fprintf(stderr, "fixdep: error opening depfile: ");
> - perror(depfile);
> - exit(2);
> - }
> - if (fstat(fd, &st) < 0) {
> - fprintf(stderr, "fixdep: error fstat'ing depfile: ");
> - perror(depfile);
> - exit(2);
> - }
> - if (st.st_size == 0) {
> - fprintf(stderr,"fixdep: %s is empty\n",depfile);
> - close(fd);
> - return;
> - }
> - map = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0);
> - if ((long) map == -1) {
> - perror("fixdep: mmap");
> - close(fd);
> - return;
> - }
> -
> - parse_dep_file(map, st.st_size);
> -
> - munmap(map, st.st_size);
> -
> - close(fd);
> -}
> -
> -static void traps(void)
> -{
> - static char test[] __attribute__((aligned(sizeof(int)))) = "CONF";
> - int *p = (int *)test;
> -
> - if (*p != INT_CONF) {
> - fprintf(stderr, "fixdep: sizeof(int) != 4 or wrong
> endianness? %#x\n",
> - *p);
> - exit(2);
> - }
> -}
> -
> int main(int argc, char *argv[])
> {
> - traps();
> -
> - if (argc != 4)
> + const char *depfile, *target, *cmdline;
> + int insert_extra_deps = 0;
> + void *buf;
> +
> + if (argc == 5 && !strcmp(argv[1], "-e")) {
> + insert_extra_deps = 1;
> + argv++;
> + } else if (argc != 4)
> usage();
>
> depfile = argv[1];
> @@ -474,8 +421,11 @@ int main(int argc, char *argv[])
> if (!strncmp(target, "spl/", 4) || !strncmp(target, "tpl/", 4))
> is_spl_build = 1;
>
> - print_cmdline();
> - print_deps();
> + printf("cmd_%s := %s\n\n", target, cmdline);
> +
> + buf = read_file(depfile);
> + parse_dep_file(buf, target, insert_extra_deps);
> + free(buf);
>
> return 0;
> }
> --
> 2.17.1
>
--
Best Regards
Masahiro Yamada
