From 7c2ec43a2154c9cd6477f85de8d247ad491cf2d1 Mon Sep 17 00:00:00 2001 From: Lukas Bulwahn Date: Mon, 8 Jan 2018 11:04:01 +0100 Subject: fixdep: exit with error code in error branches of do_config_file() do_config_file() should exit with an error code on internal run-time errors, and not return if it fails as then the error in do_config_file() would go unnoticed in the current code and allow the build to continue. The exit with error code will make the build fail in those very exceptional cases. If this occurs, this actually indicates a deeper problem in the execution of the kernel build process. Now, in these error cases, we do not explicitly free memory and close the file handlers in do_config_file(), as this is covered by exit(). This issue in the fixdep script was introduced with its initial implementation back in 2002 by the original author Kai Germaschewski with this commit 04bd72170653 ("kbuild: Make dependencies at compile time") in the linux history git tree, i.e., git://git.kernel.org/pub/scm/linux/kernel/git/history/history.git. This issue was identified during the review of a previous patch that intended to address a memory leak detected by a static analysis tool. Link: https://lkml.org/lkml/2017/12/14/736 Suggested-by: Nicholas Mc Guire Suggested-by: Masahiro Yamada Signed-off-by: Lukas Bulwahn Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index bbf62cb1f819..86a61d642220 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -290,13 +290,11 @@ static void do_config_file(const char *filename) map = malloc(st.st_size + 1); if (!map) { perror("fixdep: malloc"); - close(fd); - return; + exit(2); } if (read(fd, map, st.st_size) != st.st_size) { perror("fixdep: read"); - close(fd); - return; + exit(2); } map[st.st_size] = '\0'; close(fd); -- cgit v1.2.3 From 41f92cffba1908bc7acc847e498e34368be29dc7 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 11 Jan 2018 22:05:40 +0900 Subject: fixdep: remove unnecessary inclusion was included for ntohl(), but it was removed by commit dee81e988674 ("fixdep: faster CONFIG_ search"). Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 1 - 1 file changed, 1 deletion(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index 86a61d642220..b9b4bbf4e8dd 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -112,7 +112,6 @@ #include #include #include -#include int insert_extra_deps; char *target; -- cgit v1.2.3 From 01b5cbe7012fb1eeffc5c143865569835bcd405e Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 11 Jan 2018 22:05:41 +0900 Subject: fixdep: use malloc() and read() to load dep_file to buffer Commit dee81e988674 ("fixdep: faster CONFIG_ search") changed how to read files in which CONFIG options are searched. It used malloc() and read() instead of mmap() because it needed to zero-terminate the buffer in order to use strstr(). print_deps() was left untouched since there was no reason to change it. Now, I have two motivations to change it in the same way. - do_config_file() and print_deps() do quite similar things; they open a file, load it onto memory, and pass it to a parser function. If we use malloc() and read() for print_deps() too, we can factor out the common code. (I will do this in the next commit.) - parse_dep_file() copies each token to a temporary buffer because it needs to zero-terminate it to be passed to printf(). It is not possible to modify the buffer directly because it is mmap'ed with O_RDONLY. If we load the file content into a malloc'ed buffer, we can insert '\0' after each token, and save memcpy(). (I will do this in the commit after next.) Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 56 ++++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 24 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index b9b4bbf4e8dd..91bb4c13f121 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -104,7 +104,6 @@ #include #include -#include #include #include #include @@ -308,24 +307,27 @@ 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) { - 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; - 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 */ @@ -373,6 +375,10 @@ static void parse_dep_file(void *map, size_t len) do_config_file(s); } } + + if (is_last) + break; + /* * Start searching for next token immediately after the first * "whitespace" character that follows this token. @@ -391,40 +397,42 @@ static void parse_dep_file(void *map, size_t len) printf("$(deps_%s):\n", target); } -static void print_deps(void) +static void print_deps(const char *filename) { struct stat st; int fd; - void *map; + char *buf; - fd = open(depfile, O_RDONLY); + fd = open(filename, O_RDONLY); if (fd < 0) { fprintf(stderr, "fixdep: error opening depfile: "); - perror(depfile); + perror(filename); exit(2); } if (fstat(fd, &st) < 0) { fprintf(stderr, "fixdep: error fstat'ing depfile: "); - perror(depfile); + perror(filename); 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; + buf = malloc(st.st_size + 1); + if (!buf) { + perror("fixdep: malloc"); + exit(2); } + if (read(fd, buf, st.st_size) != st.st_size) { + perror("fixdep: read"); + exit(2); + } + buf[st.st_size] = '\0'; + close(fd); - parse_dep_file(map, st.st_size); - - munmap(map, st.st_size); + parse_dep_file(buf); - close(fd); + free(buf); } int main(int argc, char *argv[]) @@ -440,7 +448,7 @@ int main(int argc, char *argv[]) cmdline = argv[3]; print_cmdline(); - print_deps(); + print_deps(depfile); return 0; } -- cgit v1.2.3 From 4003fd80cba967e4044ebac96f13746153e87c4d Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 11 Jan 2018 22:05:42 +0900 Subject: fixdep: factor out common code for reading files Now, do_config_files() and print_deps() are almost the same. Only the difference is the parser function called (parse_config_file vs parse_dep_file). We can reduce the code duplication by factoring out the common code into read_file() - this function allocates a buffer and loads a file to it. It returns the pointer to the allocated buffer. (As before, it bails out by exit(2) for any error.) The caller must free the buffer when done. Having empty source files is possible; fixdep should simply skip them. I deleted the "st.st_size == 0" check, so read_file() allocates 1-byte buffer for an empty file. strstr() will immediately return NULL, and this is what we expect. On the other hand, an empty dep_file should be treated as an error. In this case, parse_dep_file() will error out with "no targets found" and it is a correct error message. Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 75 ++++++++++++++------------------------------------ 1 file changed, 20 insertions(+), 55 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index 91bb4c13f121..9f9238eaec19 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -264,42 +264,36 @@ static int strrcmp(const char *s, const char *sub) 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; - char *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; - } - map = malloc(st.st_size + 1); - if (!map) { + buf = malloc(st.st_size + 1); + if (!buf) { perror("fixdep: malloc"); exit(2); } - if (read(fd, map, st.st_size) != st.st_size) { + if (read(fd, buf, st.st_size) != st.st_size) { perror("fixdep: read"); exit(2); } - map[st.st_size] = '\0'; + buf[st.st_size] = '\0'; close(fd); - parse_config_file(map); - - free(map); + return buf; } /* @@ -314,6 +308,7 @@ static void parse_dep_file(char *m) int is_last, is_target; int saw_any_target = 0; int is_first_dep = 0; + void *buf; while (1) { /* Skip any "white space" */ @@ -372,7 +367,10 @@ static void parse_dep_file(char *m) is_first_dep = 0; } else printf(" %s \\\n", s); - do_config_file(s); + + buf = read_file(s); + parse_config_file(buf); + free(buf); } } @@ -397,46 +395,10 @@ static void parse_dep_file(char *m) printf("$(deps_%s):\n", target); } -static void print_deps(const char *filename) -{ - struct stat st; - int fd; - char *buf; - - fd = open(filename, O_RDONLY); - if (fd < 0) { - fprintf(stderr, "fixdep: error opening depfile: "); - perror(filename); - exit(2); - } - if (fstat(fd, &st) < 0) { - fprintf(stderr, "fixdep: error fstat'ing depfile: "); - 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); - } - if (read(fd, buf, st.st_size) != st.st_size) { - perror("fixdep: read"); - exit(2); - } - buf[st.st_size] = '\0'; - close(fd); - - parse_dep_file(buf); - - free(buf); -} - int main(int argc, char *argv[]) { + void *buf; + if (argc == 5 && !strcmp(argv[1], "-e")) { insert_extra_deps = 1; argv++; @@ -448,7 +410,10 @@ int main(int argc, char *argv[]) cmdline = argv[3]; print_cmdline(); - print_deps(depfile); + + buf = read_file(depfile); + parse_dep_file(buf); + free(buf); return 0; } -- cgit v1.2.3 From ccfe78873c22561d3c514790094dc408e4876077 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 11 Jan 2018 22:05:43 +0900 Subject: fixdep: remove unneeded memcpy() in parse_dep_file() Each token in the depfile is copied to the temporary buffer 's' to terminate the token with zero. We do not need to do this any more because the parsed buffer is now writable. Insert '\0' directly in the buffer without calling memcpy(). is no longer necessary. (It was needed for PATH_MAX). Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index 9f9238eaec19..dfba77b5d0c3 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -109,7 +109,6 @@ #include #include #include -#include #include int insert_extra_deps; @@ -304,7 +303,6 @@ static void *read_file(const char *filename) static void parse_dep_file(char *m) { char *p; - char s[PATH_MAX]; int is_last, is_target; int saw_any_target = 0; int is_first_dep = 0; @@ -330,16 +328,14 @@ static void parse_dep_file(char *m) /* 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; + *p = '\0'; /* Ignore certain dependencies */ - if (strrcmp(s, "include/generated/autoconf.h") && - strrcmp(s, "include/generated/autoksyms.h") && - strrcmp(s, "arch/um/include/uml-config.h") && - strrcmp(s, "include/linux/kconfig.h") && - strrcmp(s, ".ver")) { + if (strrcmp(m, "include/generated/autoconf.h") && + strrcmp(m, "include/generated/autoksyms.h") && + strrcmp(m, "arch/um/include/uml-config.h") && + strrcmp(m, "include/linux/kconfig.h") && + strrcmp(m, ".ver")) { /* * Do not list the source file as dependency, * so that kbuild is not confused if a .c file @@ -360,15 +356,15 @@ static void parse_dep_file(char *m) if (!saw_any_target) { saw_any_target = 1; printf("source_%s := %s\n\n", - target, s); + target, m); printf("deps_%s := \\\n", target); } is_first_dep = 0; } else - printf(" %s \\\n", s); + printf(" %s \\\n", m); - buf = read_file(s); + buf = read_file(m); parse_config_file(buf); free(buf); } -- cgit v1.2.3 From 5d1ef76f5a22ea07120671c5fcddafd365000cc9 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 11 Jan 2018 22:05:44 +0900 Subject: fixdep: move global variables to local variables of main() I do not mind global variables where they are useful enough. In this case, I do not see a good reason to use global variables since they are just referenced in shallow places. It is easy to pass them via function arguments. I squashed print_cmdline() into main() since it is just one line code. Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 42 ++++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 26 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index dfba77b5d0c3..d33b5a4c9ecb 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -111,11 +111,6 @@ #include #include -int insert_extra_deps; -char *target; -char *depfile; -char *cmdline; - static void usage(void) { fprintf(stderr, "Usage: fixdep [-e] \n"); @@ -123,14 +118,6 @@ static void usage(void) exit(1); } -/* - * Print out the commandline prefixed with cmd_ := - */ -static void print_cmdline(void) -{ - printf("cmd_%s := %s\n\n", target, cmdline); -} - /* * Print out a dependency path from a symbol name */ @@ -152,16 +139,16 @@ static void print_config(const char *m, int slen) static void do_extra_deps(void) { - if (insert_extra_deps) { - 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_config(buf, len-1); + 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_config(buf, len - 1); } } @@ -300,7 +287,7 @@ static void *read_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(char *m) +static void parse_dep_file(char *m, const char *target, int insert_extra_deps) { char *p; int is_last, is_target; @@ -385,7 +372,8 @@ static void parse_dep_file(char *m) exit(1); } - do_extra_deps(); + if (insert_extra_deps) + do_extra_deps(); printf("\n%s: $(deps_%s)\n\n", target, target); printf("$(deps_%s):\n", target); @@ -393,6 +381,8 @@ static void parse_dep_file(char *m) int main(int argc, char *argv[]) { + const char *depfile, *target, *cmdline; + int insert_extra_deps = 0; void *buf; if (argc == 5 && !strcmp(argv[1], "-e")) { @@ -405,10 +395,10 @@ int main(int argc, char *argv[]) target = argv[2]; cmdline = argv[3]; - print_cmdline(); + printf("cmd_%s := %s\n\n", target, cmdline); buf = read_file(depfile); - parse_dep_file(buf); + parse_dep_file(buf, target, insert_extra_deps); free(buf); return 0; -- cgit v1.2.3 From 87b95a81357dd6ec679a786cebfe34d0e797f752 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 11 Jan 2018 22:05:45 +0900 Subject: fixdep: refactor parse_dep_file() parse_dep_file() has too much indentation, and puts the code far to the right. This commit refactors the code and reduces the one level of indentation. strrcmp() computes 'slen' by itself, but the caller already knows the length of the token, so 'slen' can be passed via function argument. With this, we can swap the order of strrcmp() and "*p = \0;" Also, strrcmp() is an ambiguous function name. Flip the logic and rename it to str_ends_with(). I added a new helper is_ignored_file() - this returns 1 if the token represents a file that should be ignored. Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 80 +++++++++++++++++++++++++------------------------- 1 file changed, 40 insertions(+), 40 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index d33b5a4c9ecb..0abc15778f56 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -239,15 +239,14 @@ static void parse_config_file(const char *p) } /* test if s ends in sub */ -static int strrcmp(const char *s, const char *sub) +static int str_ends_with(const char *s, int slen, const char *sub) { - int slen = strlen(s); int sublen = strlen(sub); if (sublen > slen) - return 1; + return 0; - return memcmp(s + slen - sublen, sub, sublen); + return !memcmp(s + slen - sublen, sub, sublen); } static void *read_file(const char *filename) @@ -282,6 +281,16 @@ static void *read_file(const char *filename) return buf; } +/* 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, "arch/um/include/uml-config.h") || + str_ends_with(s, len, "include/linux/kconfig.h") || + str_ends_with(s, len, ".ver"); +} + /* * Important: The below generated source_foo.o and deps_foo.o variable * assignments are parsed not only by make, but also by the rather simple @@ -314,47 +323,38 @@ static void parse_dep_file(char *m, const char *target, int insert_extra_deps) if (is_target) { /* The /next/ file is the first dependency */ is_first_dep = 1; - } else { + } else if (!is_ignored_file(m, p - m)) { *p = '\0'; - /* Ignore certain dependencies */ - if (strrcmp(m, "include/generated/autoconf.h") && - strrcmp(m, "include/generated/autoksyms.h") && - strrcmp(m, "arch/um/include/uml-config.h") && - strrcmp(m, "include/linux/kconfig.h") && - strrcmp(m, ".ver")) { + /* + * 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, 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 (!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) -- cgit v1.2.3 From ab9ce9feed362f3f51e239f9421b220aaf9211aa Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 11 Jan 2018 22:05:46 +0900 Subject: fixdep: use existing helper to check modular CONFIG options str_ends_with() tests if the given token ends with a particular string. Currently, it is used to check file paths without $(srctree). Actually, we have one more place where this helper is useful. Use it to check if CONFIG option ends with _MODULE. Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index 0abc15778f56..fa3d39b6f23b 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -219,6 +219,17 @@ static void use_config(const char *m, int slen) print_config(m, slen); } +/* 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 *p) { const char *q, *r; @@ -228,7 +239,7 @@ static void parse_config_file(const char *p) q = p; while (*q && (isalnum(*q) || *q == '_')) q++; - if (memcmp(q - 7, "_MODULE", 7) == 0) + if (str_ends_with(p, q - p, "_MODULE")) r = q - 7; else r = q; @@ -238,17 +249,6 @@ static void parse_config_file(const char *p) } } -/* 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 *read_file(const char *filename) { struct stat st; -- cgit v1.2.3 From 14a596a7e6fd9c5baa6b2cfc57962e2c3bda6c69 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Wed, 28 Feb 2018 20:17:35 +0100 Subject: fixdep: remove stale references to uml-config.h uml-config.h hasn't existed in this decade (87e299e5c750 - x86, um: get rid of uml-config.h). The few remaining UML_CONFIG instances are defined directly in terms of their real CONFIG symbol in common-offsets.h, so unlike when the symbols got defined via a sed script, anything that uses UML_CONFIG_FOO now should also automatically pick up a dependency on CONFIG_FOO via the normal fixdep mechanism (since common-offsets.h should at least recursively be a dependency). Hence I believe we should actually be able to ignore the HELLO_CONFIG_BOOM cases. Cc: Al Viro Cc: Richard Weinberger Cc: user-mode-linux-devel@lists.sourceforge.net Signed-off-by: Rasmus Villemoes Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 9 --------- 1 file changed, 9 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index fa3d39b6f23b..d7fbe545dd5d 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -93,14 +93,6 @@ * (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 #include @@ -286,7 +278,6 @@ 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, "arch/um/include/uml-config.h") || str_ends_with(s, len, "include/linux/kconfig.h") || str_ends_with(s, len, ".ver"); } -- cgit v1.2.3 From 5b8ad96d1a4421ffe417e647a65064aad1e84fb4 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Wed, 28 Feb 2018 20:17:36 +0100 Subject: fixdep: remove some false CONFIG_ matches The string CONFIG_ quite often appears after other alphanumerics, meaning that that instance cannot be referencing a Kconfig symbol. Omitting these means make has fewer files to stat() when deciding what needs to be rebuilt - for a defconfig build, this seems to remove about 2% of the (wildcard ...) lines from the .o.cmd files. Signed-off-by: Rasmus Villemoes Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index d7fbe545dd5d..1b21870d6e7f 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -225,8 +225,13 @@ static int str_ends_with(const char *s, int slen, const char *sub) static void parse_config_file(const char *p) { const char *q, *r; + const char *start = p; while ((p = strstr(p, "CONFIG_"))) { + if (p > start && (isalnum(p[-1]) || p[-1] == '_')) { + p += 7; + continue; + } p += 7; q = p; while (*q && (isalnum(*q) || *q == '_')) -- cgit v1.2.3 From 638e69cf2230737655fcb5ee9879c2fab7679187 Mon Sep 17 00:00:00 2001 From: Rasmus Villemoes Date: Wed, 28 Feb 2018 20:17:37 +0100 Subject: fixdep: do not ignore kconfig.h kconfig.h was excluded from consideration by fixdep by 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false positive hits (1) include/config/.h (2) include/config/h.h (3) include/config/foo.h (1) occurred because kconfig.h contains the string CONFIG_ in a comment. However, since dee81e988674 (fixdep: faster CONFIG_ search), we have a check that the part after CONFIG_ is non-empty, so this does not happen anymore (and CONFIG_ appears by itself elsewhere, so that check is worthwhile). (2) comes from the include guard, __LINUX_KCONFIG_H. But with the previous patch, we no longer match that either. That leaves (3), which amounts to one [1] false dependency (aka stat() call done by make), which I think we can live with: We've already had one case [2] where the lack of include/linux/kconfig.h in the .o.cmd file caused a missing rebuild, and while I originally thought we should just put kconfig.h in the dependency list without parsing it for the CONFIG_ pattern, we actually do have some real CONFIG_ symbols mentioned in it, and one can imagine some translation unit that just does '#ifdef __BIG_ENDIAN' but doesn't through some other header actually depend on CONFIG_CPU_BIG_ENDIAN - so changing the target endianness could end up rebuilding the world, minus that small TU. Quoting Linus, ... when missing dependencies cause a missed re-compile, the resulting bugs can be _really_ subtle. [1] well, two, we now also have CONFIG_BOOGER/booger.h - we could change that to FOO if we care [2] https://lkml.org/lkml/2018/2/22/838 Cc: Linus Torvalds Signed-off-by: Rasmus Villemoes Signed-off-by: Masahiro Yamada --- scripts/basic/fixdep.c | 1 - 1 file changed, 1 deletion(-) (limited to 'scripts/basic/fixdep.c') diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index 1b21870d6e7f..449b68c4c90c 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -283,7 +283,6 @@ 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"); } -- cgit v1.2.3 From fbfa9be9904e2c0d0c97d860fd071089d21dd9be Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 16 Mar 2018 16:37:14 +0900 Subject: kbuild: move include/config/ksym/* to include/ksym/* The idea of using fixdep was inspired by Kconfig, but autoksyms belongs to a different group. So, I want to move those touched files under include/config/ksym/ to include/ksym/. The directory include/ksym/ can be removed by 'make clean' because it is meaningless for the external module building. Signed-off-by: Masahiro Yamada Acked-by: Nicolas Pitre --- .gitignore | 1 + Makefile | 2 +- scripts/Kbuild.include | 2 +- scripts/adjust_autoksyms.sh | 2 +- scripts/basic/fixdep.c | 8 ++++---- 5 files changed, 8 insertions(+), 7 deletions(-) (limited to 'scripts/basic/fixdep.c') diff --git a/.gitignore b/.gitignore index 1be78fd8163b..85bcc2696442 100644 --- a/.gitignore +++ b/.gitignore @@ -87,6 +87,7 @@ modules.builtin # include/config include/generated +include/ksym arch/*/include/generated # stgit generated dirs diff --git a/Makefile b/Makefile index 1e7f134dac0d..416e6b1eb4b8 100644 --- a/Makefile +++ b/Makefile @@ -1336,7 +1336,7 @@ endif # CONFIG_MODULES # make distclean Remove editor backup files, patch leftover files and the like # Directories & files removed with 'make clean' -CLEAN_DIRS += $(MODVERDIR) +CLEAN_DIRS += $(MODVERDIR) include/ksym # Directories & files removed with 'make mrproper' MRPROPER_DIRS += include/config usr/include include/generated \ diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index f9c2f07442c5..cce31ee876b6 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -366,7 +366,7 @@ ksym_dep_filter = \ $(CPP) $(call flags_nodeps,a_flags) -D__KSYM_DEPS__ $< ;; \ boot*|build*|cpp_its_S|*cpp_lds_S|dtc|host*|vdso*) : ;; \ *) echo "Don't know how to preprocess $(1)" >&2; false ;; \ - esac | tr ";" "\n" | sed -n 's/^.*=== __KSYM_\(.*\) ===.*$$/KSYM_\1/p' + esac | tr ";" "\n" | sed -n 's/^.*=== __KSYM_\(.*\) ===.*$$/_\1/p' cmd_and_fixdep = \ $(echo-cmd) $(cmd_$(1)); \ diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh index e0dd0d5148ec..f11cae6a041b 100755 --- a/scripts/adjust_autoksyms.sh +++ b/scripts/adjust_autoksyms.sh @@ -80,7 +80,7 @@ sort "$cur_ksyms_file" "$new_ksyms_file" | uniq -u | sed -n 's/^#define __KSYM_\(.*\) 1/\1/p' | tr "A-Z_" "a-z/" | while read sympath; do if [ -z "$sympath" ]; then continue; fi - depfile="include/config/ksym/${sympath}.h" + depfile="include/ksym/${sympath}.h" mkdir -p "$(dirname "$depfile")" touch "$depfile" echo $((count += 1)) diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index 449b68c4c90c..f387538c58bc 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -113,11 +113,11 @@ static void usage(void) /* * Print out a dependency path from a symbol name */ -static void print_config(const char *m, int slen) +static void print_dep(const char *m, int slen, const char *dir) { int c, i; - printf(" $(wildcard include/config/"); + printf(" $(wildcard %s/", dir); for (i = 0; i < slen; i++) { c = m[i]; if (c == '_') @@ -140,7 +140,7 @@ static void do_extra_deps(void) fprintf(stderr, "fixdep: bad data on stdin\n"); exit(1); } - print_config(buf, len - 1); + print_dep(buf, len - 1, "include/ksym"); } } @@ -208,7 +208,7 @@ static void use_config(const char *m, int slen) return; define_config(m, slen, hash); - print_config(m, slen); + print_dep(m, slen, "include/config"); } /* test if s ends in sub */ -- cgit v1.2.3