* [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple()
@ 2018-11-30 9:15 Masahiro Yamada
2018-11-30 9:15 ` [PATCH 2/5] kconfig: rename conf_split_config() to conf_touch_deps() Masahiro Yamada
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Masahiro Yamada @ 2018-11-30 9:15 UTC (permalink / raw
To: linux-kbuild; +Cc: Sam Ravnborg, Ulf Magnusson, Masahiro Yamada, linux-kernel
The two 'goto setsym' statements are reachable only when sym == NULL.
The code below the 'setsym:' label does nothing when sym == NULL
since there is just one if-block guarded by 'if (sym && ...)'.
Hence, 'goto setsym' can be replaced with 'continue'.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/confdata.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 91d0a5c..1e35529 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -363,7 +363,7 @@ int conf_read_simple(const char *name, int def)
sym = sym_find(line + 2 + strlen(CONFIG_));
if (!sym) {
sym_add_change_count(1);
- goto setsym;
+ continue;
}
} else {
sym = sym_lookup(line + 2 + strlen(CONFIG_), 0);
@@ -397,7 +397,7 @@ int conf_read_simple(const char *name, int def)
sym = sym_find(line + strlen(CONFIG_));
if (!sym) {
sym_add_change_count(1);
- goto setsym;
+ continue;
}
} else {
sym = sym_lookup(line + strlen(CONFIG_), 0);
@@ -416,7 +416,7 @@ int conf_read_simple(const char *name, int def)
continue;
}
-setsym:
+
if (sym && sym_is_choice_value(sym)) {
struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
switch (sym->def[def].tri) {
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/5] kconfig: rename conf_split_config() to conf_touch_deps()
2018-11-30 9:15 [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
@ 2018-11-30 9:15 ` Masahiro Yamada
2018-11-30 9:15 ` [PATCH 3/5] kconfig: split out code touching a file to conf_touch_dep() Masahiro Yamada
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2018-11-30 9:15 UTC (permalink / raw
To: linux-kbuild; +Cc: Sam Ravnborg, Ulf Magnusson, Masahiro Yamada, linux-kernel
According to commit 2e3646e51b2d ("kconfig: integrate split config
into silentoldconfig"), this function was named after split-include
tool, which used to exist in old versions of Linux.
Setting aside the historical reason, rename it into a more intuitive
name. This function touches timestamp files under include/config/
in order to interact with the fixdep tool.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/confdata.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 1e35529..4c76d56 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -906,7 +906,7 @@ static int conf_write_dep(const char *name)
return 0;
}
-static int conf_split_config(void)
+static int conf_touch_deps(void)
{
const char *name;
char path[PATH_MAX+1];
@@ -1028,7 +1028,7 @@ int conf_write_autoconf(int overwrite)
conf_write_dep("include/config/auto.conf.cmd");
- if (conf_split_config())
+ if (conf_touch_deps())
return 1;
out = fopen(".tmpconfig", "w");
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/5] kconfig: split out code touching a file to conf_touch_dep()
2018-11-30 9:15 [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
2018-11-30 9:15 ` [PATCH 2/5] kconfig: rename conf_split_config() to conf_touch_deps() Masahiro Yamada
@ 2018-11-30 9:15 ` Masahiro Yamada
2018-11-30 9:15 ` [PATCH 4/5] kconfig: remove S_OTHER symbol type and correct dependency tracking Masahiro Yamada
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2018-11-30 9:15 UTC (permalink / raw
To: linux-kbuild; +Cc: Sam Ravnborg, Ulf Magnusson, Masahiro Yamada, linux-kernel
conf_touch_deps() iterates over symbols, touching corresponding
include/config/*.h files as needed.
Split the part that touches a single file into a new helper so it can
be reused.
The new helper, conf_touch_dep(), takes a symbol name as a parameter,
and touches the corresponding include/config/*.h file.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/confdata.c | 92 ++++++++++++++++++++++++----------------------
1 file changed, 49 insertions(+), 43 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 4c76d56..7263b83 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -74,6 +74,47 @@ static int make_parent_dir(const char *path)
return 0;
}
+static char depfile_path[PATH_MAX];
+static size_t depfile_prefix_len;
+
+/* touch depfile for symbol 'name' */
+static int conf_touch_dep(const char *name)
+{
+ int fd, ret;
+ const char *s;
+ char *d, c;
+
+ /* check overflow: prefix + name + ".h" + '\0' must fix in buffer. */
+ if (depfile_prefix_len + strlen(name) + 3 > sizeof(depfile_path))
+ return -1;
+
+ d = depfile_path + depfile_prefix_len;
+ s = name;
+
+ while ((c = *s++))
+ *d++ = (c == '_') ? '/' : tolower(c);
+ strcpy(d, ".h");
+
+ /* Assume directory path already exists. */
+ fd = open(depfile_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+ if (fd == -1) {
+ if (errno != ENOENT)
+ return -1;
+
+ ret = make_parent_dir(depfile_path);
+ if (ret)
+ return ret;
+
+ /* Try it again. */
+ fd = open(depfile_path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+ if (fd == -1)
+ return -1;
+ }
+ close(fd);
+
+ return 0;
+}
+
struct conf_printer {
void (*print_symbol)(FILE *, struct symbol *, const char *, void *);
void (*print_comment)(FILE *, const char *, void *);
@@ -909,21 +950,16 @@ static int conf_write_dep(const char *name)
static int conf_touch_deps(void)
{
const char *name;
- char path[PATH_MAX+1];
- char *s, *d, c;
struct symbol *sym;
- int res, i, fd;
+ int res, i;
+
+ strcpy(depfile_path, "include/config/");
+ depfile_prefix_len = strlen(depfile_path);
name = conf_get_autoconfig_name();
conf_read_simple(name, S_DEF_AUTO);
sym_calc_value(modules_sym);
- if (make_parent_dir("include/config/foo.h"))
- return 1;
- if (chdir("include/config"))
- return 1;
-
- res = 0;
for_all_symbols(i, sym) {
sym_calc_value(sym);
if ((sym->flags & SYMBOL_NO_WRITE) || !sym->name)
@@ -975,42 +1011,12 @@ static int conf_touch_deps(void)
* different from 'no').
*/
- /* Replace all '_' and append ".h" */
- s = sym->name;
- d = path;
- while ((c = *s++)) {
- c = tolower(c);
- *d++ = (c == '_') ? '/' : c;
- }
- strcpy(d, ".h");
-
- /* Assume directory path already exists. */
- fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
- if (fd == -1) {
- if (errno != ENOENT) {
- res = 1;
- break;
- }
-
- if (make_parent_dir(path)) {
- res = 1;
- goto out;
- }
-
- /* Try it again. */
- fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
- if (fd == -1) {
- res = 1;
- break;
- }
- }
- close(fd);
+ res = conf_touch_dep(sym->name);
+ if (res)
+ return res;
}
-out:
- if (chdir("../.."))
- return 1;
- return res;
+ return 0;
}
int conf_write_autoconf(int overwrite)
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/5] kconfig: remove S_OTHER symbol type and correct dependency tracking
2018-11-30 9:15 [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
2018-11-30 9:15 ` [PATCH 2/5] kconfig: rename conf_split_config() to conf_touch_deps() Masahiro Yamada
2018-11-30 9:15 ` [PATCH 3/5] kconfig: split out code touching a file to conf_touch_dep() Masahiro Yamada
@ 2018-11-30 9:15 ` Masahiro Yamada
2018-11-30 9:15 ` [PATCH 5/5] kconfig: remove k_invalid from expr_parse_string() return type Masahiro Yamada
2018-12-08 6:55 ` [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
4 siblings, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2018-11-30 9:15 UTC (permalink / raw
To: linux-kbuild; +Cc: Sam Ravnborg, Ulf Magnusson, Masahiro Yamada, linux-kernel
The S_OTHER type could be set only when conf_read_simple() is reading
include/config/auto.conf file.
For example, CONFIG_FOO=y exists in include/config/auto.conf but it is
missing from the currently parsed Kconfig files, sym_lookup() allocates
a new symbol, and sets its type to S_OTHER.
Strangely, it will be set to S_STRING by conf_set_sym_val() a few lines
below while it is obviously bool or tristate type. On the other hand,
when CONFIG_BAR="bar" is being dropped from include/config/auto.conf,
its type remains S_OTHER. Because for_all_symbols() omits S_OTHER
symbols, conf_touch_deps() misses to touch include/config/bar.h
This behavior has been a pretty mystery for me, and digging the git
histroy did not help. At least, touching depfiles is broken for string
type symbols.
I removed S_OTHER entirely, and reimplemented it more simply.
If CONFIG_FOO was visible in the previous syncconfig, but is missing
now, what we want to do is quite simple; just call conf_touch_dep()
to touch include/config/foo.h instead of allocating a new symbol data.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/confdata.c | 33 ++++++++++++++-------------------
scripts/kconfig/expr.h | 4 ++--
scripts/kconfig/symbol.c | 3 ---
3 files changed, 16 insertions(+), 24 deletions(-)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 7263b83..800fb16 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -227,14 +227,6 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
conf_warning("symbol value '%s' invalid for %s",
p, sym->name);
return 1;
- case S_OTHER:
- if (*p != '"') {
- for (p2 = p; *p2 && !isspace(*p2); p2++)
- ;
- sym->type = S_STRING;
- goto done;
- }
- /* fall through */
case S_STRING:
if (*p++ != '"')
break;
@@ -253,7 +245,6 @@ static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
/* fall through */
case S_INT:
case S_HEX:
- done:
if (sym_string_valid(sym, p)) {
sym->def[def].val = xstrdup(p);
sym->flags |= def_flags;
@@ -434,17 +425,22 @@ int conf_read_simple(const char *name, int def)
if (*p2 == '\r')
*p2 = 0;
}
- if (def == S_DEF_USER) {
- sym = sym_find(line + strlen(CONFIG_));
- if (!sym) {
+
+ sym = sym_find(line + strlen(CONFIG_));
+ if (!sym) {
+ if (def == S_DEF_AUTO)
+ /*
+ * Reading from include/config/auto.conf
+ * If CONFIG_FOO previously existed in
+ * auto.conf but it is missing now,
+ * include/config/foo.h must be touched.
+ */
+ conf_touch_dep(line + strlen(CONFIG_));
+ else
sym_add_change_count(1);
- continue;
- }
- } else {
- sym = sym_lookup(line + strlen(CONFIG_), 0);
- if (sym->type == S_UNKNOWN)
- sym->type = S_OTHER;
+ continue;
}
+
if (sym->flags & def_flags) {
conf_warning("override: reassigning to symbol %s", sym->name);
}
@@ -710,7 +706,6 @@ static void conf_write_symbol(FILE *fp, struct symbol *sym,
const char *str;
switch (sym->type) {
- case S_OTHER:
case S_UNKNOWN:
break;
case S_STRING:
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 7c329e1..2b7e222 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -62,7 +62,7 @@ struct symbol_value {
};
enum symbol_type {
- S_UNKNOWN, S_BOOLEAN, S_TRISTATE, S_INT, S_HEX, S_STRING, S_OTHER
+ S_UNKNOWN, S_BOOLEAN, S_TRISTATE, S_INT, S_HEX, S_STRING
};
/* enum values are used as index to symbol.def[] */
@@ -131,7 +131,7 @@ struct symbol {
struct expr_value implied;
};
-#define for_all_symbols(i, sym) for (i = 0; i < SYMBOL_HASHSIZE; i++) for (sym = symbol_hash[i]; sym; sym = sym->next) if (sym->type != S_OTHER)
+#define for_all_symbols(i, sym) for (i = 0; i < SYMBOL_HASHSIZE; i++) for (sym = symbol_hash[i]; sym; sym = sym->next)
#define SYMBOL_CONST 0x0001 /* symbol is const */
#define SYMBOL_CHECK 0x0008 /* used during dependency checking */
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 703b9b8..2e6bf36 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -61,8 +61,6 @@ const char *sym_type_name(enum symbol_type type)
return "string";
case S_UNKNOWN:
return "unknown";
- case S_OTHER:
- break;
}
return "???";
}
@@ -757,7 +755,6 @@ const char *sym_get_string_default(struct symbol *sym)
return str;
case S_STRING:
return str;
- case S_OTHER:
case S_UNKNOWN:
break;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 5/5] kconfig: remove k_invalid from expr_parse_string() return type
2018-11-30 9:15 [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
` (2 preceding siblings ...)
2018-11-30 9:15 ` [PATCH 4/5] kconfig: remove S_OTHER symbol type and correct dependency tracking Masahiro Yamada
@ 2018-11-30 9:15 ` Masahiro Yamada
2018-12-08 6:55 ` [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
4 siblings, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2018-11-30 9:15 UTC (permalink / raw
To: linux-kbuild; +Cc: Sam Ravnborg, Ulf Magnusson, Masahiro Yamada, linux-kernel
The only possibility of k_invalid being returned was when
expr_parse_sting() parsed S_OTHER type symbol. This actually never
happened, and this is even clearer since S_OTHER has gone.
Clean up unreachable code.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
scripts/kconfig/expr.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index e1a39e9..57ebf71 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -980,7 +980,6 @@ enum string_value_kind {
k_string,
k_signed,
k_unsigned,
- k_invalid
};
union string_value {
@@ -1011,13 +1010,10 @@ static enum string_value_kind expr_parse_string(const char *str,
val->u = strtoull(str, &tail, 16);
kind = k_unsigned;
break;
- case S_STRING:
- case S_UNKNOWN:
+ default:
val->s = strtoll(str, &tail, 0);
kind = k_signed;
break;
- default:
- return k_invalid;
}
return !errno && !*tail && tail > str && isxdigit(tail[-1])
? kind : k_string;
@@ -1073,13 +1069,7 @@ tristate expr_calc_value(struct expr *e)
if (k1 == k_string || k2 == k_string)
res = strcmp(str1, str2);
- else if (k1 == k_invalid || k2 == k_invalid) {
- if (e->type != E_EQUAL && e->type != E_UNEQUAL) {
- printf("Cannot compare \"%s\" and \"%s\"\n", str1, str2);
- return no;
- }
- res = strcmp(str1, str2);
- } else if (k1 == k_unsigned || k2 == k_unsigned)
+ else if (k1 == k_unsigned || k2 == k_unsigned)
res = (lval.u > rval.u) - (lval.u < rval.u);
else /* if (k1 == k_signed && k2 == k_signed) */
res = (lval.s > rval.s) - (lval.s < rval.s);
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple()
2018-11-30 9:15 [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
` (3 preceding siblings ...)
2018-11-30 9:15 ` [PATCH 5/5] kconfig: remove k_invalid from expr_parse_string() return type Masahiro Yamada
@ 2018-12-08 6:55 ` Masahiro Yamada
4 siblings, 0 replies; 6+ messages in thread
From: Masahiro Yamada @ 2018-12-08 6:55 UTC (permalink / raw
To: Linux Kbuild mailing list
Cc: Sam Ravnborg, Ulf Magnusson, Linux Kernel Mailing List
On Fri, Nov 30, 2018 at 6:17 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> The two 'goto setsym' statements are reachable only when sym == NULL.
>
> The code below the 'setsym:' label does nothing when sym == NULL
> since there is just one if-block guarded by 'if (sym && ...)'.
>
> Hence, 'goto setsym' can be replaced with 'continue'.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
Series, applied to linux-kbuild/kconfig.
> scripts/kconfig/confdata.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 91d0a5c..1e35529 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -363,7 +363,7 @@ int conf_read_simple(const char *name, int def)
> sym = sym_find(line + 2 + strlen(CONFIG_));
> if (!sym) {
> sym_add_change_count(1);
> - goto setsym;
> + continue;
> }
> } else {
> sym = sym_lookup(line + 2 + strlen(CONFIG_), 0);
> @@ -397,7 +397,7 @@ int conf_read_simple(const char *name, int def)
> sym = sym_find(line + strlen(CONFIG_));
> if (!sym) {
> sym_add_change_count(1);
> - goto setsym;
> + continue;
> }
> } else {
> sym = sym_lookup(line + strlen(CONFIG_), 0);
> @@ -416,7 +416,7 @@ int conf_read_simple(const char *name, int def)
>
> continue;
> }
> -setsym:
> +
> if (sym && sym_is_choice_value(sym)) {
> struct symbol *cs = prop_get_symbol(sym_get_choice_prop(sym));
> switch (sym->def[def].tri) {
> --
> 2.7.4
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-12-08 6:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-30 9:15 [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
2018-11-30 9:15 ` [PATCH 2/5] kconfig: rename conf_split_config() to conf_touch_deps() Masahiro Yamada
2018-11-30 9:15 ` [PATCH 3/5] kconfig: split out code touching a file to conf_touch_dep() Masahiro Yamada
2018-11-30 9:15 ` [PATCH 4/5] kconfig: remove S_OTHER symbol type and correct dependency tracking Masahiro Yamada
2018-11-30 9:15 ` [PATCH 5/5] kconfig: remove k_invalid from expr_parse_string() return type Masahiro Yamada
2018-12-08 6:55 ` [PATCH 1/5] kconfig: remove unneeded setsym label in conf_read_simple() Masahiro Yamada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).