Re: [PATCH v2] scripts/kconfig: cleanup symbol handling code

2018-03-25 Thread Joey Pabalinas 
On Mon, Mar 26, 2018 at 01:52:26AM +0900, Masahiro Yamada wrote:
> I want to see Kconfig improvements in a bigger picture.
> 
> The changes below are noise.

That's understandable; I do agree that nothing here is _fundamentally_
broken at all, so no worries.

-- 
Cheers,
Joey Pabalinas


signature.asc
Description: PGP signature

Re: [PATCH v2] scripts/kconfig: cleanup symbol handling code

2018-03-25 Thread Joey Pabalinas 
On Mon, Mar 26, 2018 at 01:52:26AM +0900, Masahiro Yamada wrote:
> I want to see Kconfig improvements in a bigger picture.
> 
> The changes below are noise.

That's understandable; I do agree that nothing here is _fundamentally_
broken at all, so no worries.

-- 
Cheers,
Joey Pabalinas


signature.asc
Description: PGP signature

Re: [PATCH v2] scripts/kconfig: cleanup symbol handling code

2018-03-25 Thread Masahiro Yamada 
2018-03-10 19:56 GMT+09:00 Joey Pabalinas :
> Many of the variable names in scripts/kconfig/symbol.c are
> far too terse to the point of not at all identifying _what_
> they are actually used for (`p` and `l` as a couple examples),
> and overall there is a large amount of code that could use
> some cleaning up.
>
> Give more explicit names to these variables, fix a couple cases
> where different variables were sharing the same name and shadowing
> each other, and overall cleanup a bit of the messiness in
> sym_expand_string_value() and sym_escape_string_value()
> while maintaining equivalent program behavior.
>
> Suggested-by: Ulf Magnusson 
> Signed-off-by: Joey Pabalinas 
>
>  1 file changed, 69 insertions(+), 61 deletions(-)


First of all, I can not compile Kconfig with this patch.


masahiro@grover:~/workspace/linux$ make defconfig
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/zconf.tab.o: In function `sym_expand_string_value':
zconf.tab.c:(.text+0x81a2): undefined reference to `strscpy'
zconf.tab.c:(.text+0x8287): undefined reference to `strscpy'
zconf.tab.c:(.text+0x82b5): undefined reference to `strscpy'
scripts/kconfig/zconf.tab.o: In function `sym_escape_string_value':
zconf.tab.c:(.text+0x869b): undefined reference to `strscpy'
scripts/kconfig/zconf.tab.o: In function `menu_add_option':
zconf.tab.c:(.text+0x99b7): undefined reference to `likely'
collect2: error: ld returned 1 exit status
scripts/Makefile.host:99: recipe for target 'scripts/kconfig/conf' failed
make[1]: *** [scripts/kconfig/conf] Error 1
Makefile:512: recipe for target 'defconfig' failed
make: *** [defconfig] Error 2






> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 2220bc4b051bd914e3..9ee32ddb44e193719c 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -5,8 +5,8 @@
>
>  #include 
>  #include 
> -#include 
>  #include 
> +#include 
>  #include 


 is correct.



I want to see Kconfig improvements in a bigger picture.

The changes below are noise.



>  #include "lkc.h"
> @@ -337,7 +337,7 @@ void sym_calc_value(struct symbol *sym)
>  {
> struct symbol_value newval, oldval;
> struct property *prop;
> -   struct expr *e;
> +   struct expr *expr;
>
> if (!sym)
> return;
> @@ -469,7 +469,7 @@ void sym_calc_value(struct symbol *sym)
> struct symbol *choice_sym;
>
> prop = sym_get_choice_prop(sym);
> -   expr_list_for_each_sym(prop->expr, e, choice_sym) {
> +   expr_list_for_each_sym(prop->expr, expr, choice_sym) {
> if ((sym->flags & SYMBOL_WRITE) &&
> choice_sym->visible != no)
> choice_sym->flags |= SYMBOL_WRITE;
> @@ -899,94 +899,100 @@ struct symbol *sym_find(const char *name)
>   * name to be expanded shall be prefixed by a '$'. Unknown symbol expands to
>   * the empty string.
>   */
> -char *sym_expand_string_value(const char *in)
> +char *sym_expand_string_value(const char *src)
>  {
> -   const char *src;
> -   char *res;
> -   size_t reslen;
> +   const char *in;
> +   char *res, *out;
> +   size_t res_len, src_len;
>
> /*
> -* Note: 'in' might come from a token that's about to be
> +* Note: 'src' might come from a token that'src about to be
>  * freed, so make sure to always allocate a new string
>  */
> -   reslen = strlen(in) + 1;
> -   res = xmalloc(reslen);
> -   res[0] = '\0';
> +   res_len = strlen(src) + 1;
> +   res = xmalloc(res_len);
> +   out = res;
>
> -   while ((src = strchr(in, '$'))) {
> +   while ((in = strchr(src, '$'))) {
> char *p, name[SYMBOL_MAXLENGTH];
> -   const char *symval = "";
> +   const char *sym_val = "";
> struct symbol *sym;
> -   size_t newlen;
> +   size_t new_len, sym_len;
>
> -   strncat(res, in, src - in);
> -   src++;
> +   strscpy(out, src, in - src);
> +   out += in - src;
> +   in++;
>
> p = name;
> -   while (isalnum(*src) || *src == '_')
> -   *p++ = *src++;
> +   while (isalnum(*in) || *in == '_')
> +   *p++ = *in++;
> *p = '\0';
>
> sym = sym_find(name);
> if (sym != NULL) {
> sym_calc_value(sym);
> -   symval = sym_get_string_value(sym);
> +   sym_val = sym_get_string_value(sym);
> }
>
> -   newlen = strlen(res) + strlen(symval) + strlen(src) + 1;
> -   if (newlen > reslen) {
> -   reslen = newlen;
> -   res = xrealloc(res, reslen);
> +   sym_len =

Re: [PATCH v2] scripts/kconfig: cleanup symbol handling code

2018-03-25 Thread Masahiro Yamada 
2018-03-10 19:56 GMT+09:00 Joey Pabalinas :
> Many of the variable names in scripts/kconfig/symbol.c are
> far too terse to the point of not at all identifying _what_
> they are actually used for (`p` and `l` as a couple examples),
> and overall there is a large amount of code that could use
> some cleaning up.
>
> Give more explicit names to these variables, fix a couple cases
> where different variables were sharing the same name and shadowing
> each other, and overall cleanup a bit of the messiness in
> sym_expand_string_value() and sym_escape_string_value()
> while maintaining equivalent program behavior.
>
> Suggested-by: Ulf Magnusson 
> Signed-off-by: Joey Pabalinas 
>
>  1 file changed, 69 insertions(+), 61 deletions(-)


First of all, I can not compile Kconfig with this patch.


masahiro@grover:~/workspace/linux$ make defconfig
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/zconf.tab.o: In function `sym_expand_string_value':
zconf.tab.c:(.text+0x81a2): undefined reference to `strscpy'
zconf.tab.c:(.text+0x8287): undefined reference to `strscpy'
zconf.tab.c:(.text+0x82b5): undefined reference to `strscpy'
scripts/kconfig/zconf.tab.o: In function `sym_escape_string_value':
zconf.tab.c:(.text+0x869b): undefined reference to `strscpy'
scripts/kconfig/zconf.tab.o: In function `menu_add_option':
zconf.tab.c:(.text+0x99b7): undefined reference to `likely'
collect2: error: ld returned 1 exit status
scripts/Makefile.host:99: recipe for target 'scripts/kconfig/conf' failed
make[1]: *** [scripts/kconfig/conf] Error 1
Makefile:512: recipe for target 'defconfig' failed
make: *** [defconfig] Error 2






> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 2220bc4b051bd914e3..9ee32ddb44e193719c 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -5,8 +5,8 @@
>
>  #include 
>  #include 
> -#include 
>  #include 
> +#include 
>  #include 


 is correct.



I want to see Kconfig improvements in a bigger picture.

The changes below are noise.



>  #include "lkc.h"
> @@ -337,7 +337,7 @@ void sym_calc_value(struct symbol *sym)
>  {
> struct symbol_value newval, oldval;
> struct property *prop;
> -   struct expr *e;
> +   struct expr *expr;
>
> if (!sym)
> return;
> @@ -469,7 +469,7 @@ void sym_calc_value(struct symbol *sym)
> struct symbol *choice_sym;
>
> prop = sym_get_choice_prop(sym);
> -   expr_list_for_each_sym(prop->expr, e, choice_sym) {
> +   expr_list_for_each_sym(prop->expr, expr, choice_sym) {
> if ((sym->flags & SYMBOL_WRITE) &&
> choice_sym->visible != no)
> choice_sym->flags |= SYMBOL_WRITE;
> @@ -899,94 +899,100 @@ struct symbol *sym_find(const char *name)
>   * name to be expanded shall be prefixed by a '$'. Unknown symbol expands to
>   * the empty string.
>   */
> -char *sym_expand_string_value(const char *in)
> +char *sym_expand_string_value(const char *src)
>  {
> -   const char *src;
> -   char *res;
> -   size_t reslen;
> +   const char *in;
> +   char *res, *out;
> +   size_t res_len, src_len;
>
> /*
> -* Note: 'in' might come from a token that's about to be
> +* Note: 'src' might come from a token that'src about to be
>  * freed, so make sure to always allocate a new string
>  */
> -   reslen = strlen(in) + 1;
> -   res = xmalloc(reslen);
> -   res[0] = '\0';
> +   res_len = strlen(src) + 1;
> +   res = xmalloc(res_len);
> +   out = res;
>
> -   while ((src = strchr(in, '$'))) {
> +   while ((in = strchr(src, '$'))) {
> char *p, name[SYMBOL_MAXLENGTH];
> -   const char *symval = "";
> +   const char *sym_val = "";
> struct symbol *sym;
> -   size_t newlen;
> +   size_t new_len, sym_len;
>
> -   strncat(res, in, src - in);
> -   src++;
> +   strscpy(out, src, in - src);
> +   out += in - src;
> +   in++;
>
> p = name;
> -   while (isalnum(*src) || *src == '_')
> -   *p++ = *src++;
> +   while (isalnum(*in) || *in == '_')
> +   *p++ = *in++;
> *p = '\0';
>
> sym = sym_find(name);
> if (sym != NULL) {
> sym_calc_value(sym);
> -   symval = sym_get_string_value(sym);
> +   sym_val = sym_get_string_value(sym);
> }
>
> -   newlen = strlen(res) + strlen(symval) + strlen(src) + 1;
> -   if (newlen > reslen) {
> -   reslen = newlen;
> -   res = xrealloc(res, reslen);
> +   sym_len = strlen(sym_val);
> +   new_len = sym_len + strlen(res) +

[PATCH v2] scripts/kconfig: cleanup symbol handling code

2018-03-10 Thread Joey Pabalinas 
Many of the variable names in scripts/kconfig/symbol.c are
far too terse to the point of not at all identifying _what_
they are actually used for (`p` and `l` as a couple examples),
and overall there is a large amount of code that could use
some cleaning up.

Give more explicit names to these variables, fix a couple cases
where different variables were sharing the same name and shadowing
each other, and overall cleanup a bit of the messiness in
sym_expand_string_value() and sym_escape_string_value()
while maintaining equivalent program behavior.

Suggested-by: Ulf Magnusson 
Signed-off-by: Joey Pabalinas 

 1 file changed, 69 insertions(+), 61 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 2220bc4b051bd914e3..9ee32ddb44e193719c 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -5,8 +5,8 @@
 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include 
 
 #include "lkc.h"
@@ -337,7 +337,7 @@ void sym_calc_value(struct symbol *sym)
 {
struct symbol_value newval, oldval;
struct property *prop;
-   struct expr *e;
+   struct expr *expr;
 
if (!sym)
return;
@@ -469,7 +469,7 @@ void sym_calc_value(struct symbol *sym)
struct symbol *choice_sym;
 
prop = sym_get_choice_prop(sym);
-   expr_list_for_each_sym(prop->expr, e, choice_sym) {
+   expr_list_for_each_sym(prop->expr, expr, choice_sym) {
if ((sym->flags & SYMBOL_WRITE) &&
choice_sym->visible != no)
choice_sym->flags |= SYMBOL_WRITE;
@@ -899,94 +899,100 @@ struct symbol *sym_find(const char *name)
  * name to be expanded shall be prefixed by a '$'. Unknown symbol expands to
  * the empty string.
  */
-char *sym_expand_string_value(const char *in)
+char *sym_expand_string_value(const char *src)
 {
-   const char *src;
-   char *res;
-   size_t reslen;
+   const char *in;
+   char *res, *out;
+   size_t res_len, src_len;
 
/*
-* Note: 'in' might come from a token that's about to be
+* Note: 'src' might come from a token that'src about to be
 * freed, so make sure to always allocate a new string
 */
-   reslen = strlen(in) + 1;
-   res = xmalloc(reslen);
-   res[0] = '\0';
+   res_len = strlen(src) + 1;
+   res = xmalloc(res_len);
+   out = res;
 
-   while ((src = strchr(in, '$'))) {
+   while ((in = strchr(src, '$'))) {
char *p, name[SYMBOL_MAXLENGTH];
-   const char *symval = "";
+   const char *sym_val = "";
struct symbol *sym;
-   size_t newlen;
+   size_t new_len, sym_len;
 
-   strncat(res, in, src - in);
-   src++;
+   strscpy(out, src, in - src);
+   out += in - src;
+   in++;
 
p = name;
-   while (isalnum(*src) || *src == '_')
-   *p++ = *src++;
+   while (isalnum(*in) || *in == '_')
+   *p++ = *in++;
*p = '\0';
 
sym = sym_find(name);
if (sym != NULL) {
sym_calc_value(sym);
-   symval = sym_get_string_value(sym);
+   sym_val = sym_get_string_value(sym);
}
 
-   newlen = strlen(res) + strlen(symval) + strlen(src) + 1;
-   if (newlen > reslen) {
-   reslen = newlen;
-   res = xrealloc(res, reslen);
+   sym_len = strlen(sym_val);
+   new_len = sym_len + strlen(res) + strlen(in) + 1;
+   if (new_len > res_len) {
+   res_len = new_len;
+   res = xrealloc(res, res_len);
}
 
-   strcat(res, symval);
-   in = src;
+   strscpy(out, sym_val, sym_len);
+   out += sym_len;
+   src = in;
}
-   strcat(res, in);
+   src_len = strlen(src);
+   strscpy(out, src, src_len);
+   out += src_len;
+   *out = '\0';
 
return res;
 }
 
-const char *sym_escape_string_value(const char *in)
+const char *sym_escape_string_value(const char *src)
 {
-   const char *p;
-   size_t reslen;
-   char *res;
-   size_t l;
+   const char *in;
+   size_t res_len, in_len;
+   char *res, *out;
 
-   reslen = strlen(in) + strlen("\"\"") + 1;
+   res_len = strlen(src) + strlen("\"\"") + 1;
 
-   p = in;
+   in = src;
for (;;) {
-   l = strcspn(p, "\"\\");
-   p += l;
+   in_len = strcspn(in, "\"\\");
+   in += in_len;
 
-   if (p[0] == '\0')
+   if (*in == '\0')
break;
 
-

[PATCH v2] scripts/kconfig: cleanup symbol handling code

2018-03-10 Thread Joey Pabalinas 
Many of the variable names in scripts/kconfig/symbol.c are
far too terse to the point of not at all identifying _what_
they are actually used for (`p` and `l` as a couple examples),
and overall there is a large amount of code that could use
some cleaning up.

Give more explicit names to these variables, fix a couple cases
where different variables were sharing the same name and shadowing
each other, and overall cleanup a bit of the messiness in
sym_expand_string_value() and sym_escape_string_value()
while maintaining equivalent program behavior.

Suggested-by: Ulf Magnusson 
Signed-off-by: Joey Pabalinas 

 1 file changed, 69 insertions(+), 61 deletions(-)

diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 2220bc4b051bd914e3..9ee32ddb44e193719c 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -5,8 +5,8 @@
 
 #include 
 #include 
-#include 
 #include 
+#include 
 #include 
 
 #include "lkc.h"
@@ -337,7 +337,7 @@ void sym_calc_value(struct symbol *sym)
 {
struct symbol_value newval, oldval;
struct property *prop;
-   struct expr *e;
+   struct expr *expr;
 
if (!sym)
return;
@@ -469,7 +469,7 @@ void sym_calc_value(struct symbol *sym)
struct symbol *choice_sym;
 
prop = sym_get_choice_prop(sym);
-   expr_list_for_each_sym(prop->expr, e, choice_sym) {
+   expr_list_for_each_sym(prop->expr, expr, choice_sym) {
if ((sym->flags & SYMBOL_WRITE) &&
choice_sym->visible != no)
choice_sym->flags |= SYMBOL_WRITE;
@@ -899,94 +899,100 @@ struct symbol *sym_find(const char *name)
  * name to be expanded shall be prefixed by a '$'. Unknown symbol expands to
  * the empty string.
  */
-char *sym_expand_string_value(const char *in)
+char *sym_expand_string_value(const char *src)
 {
-   const char *src;
-   char *res;
-   size_t reslen;
+   const char *in;
+   char *res, *out;
+   size_t res_len, src_len;
 
/*
-* Note: 'in' might come from a token that's about to be
+* Note: 'src' might come from a token that'src about to be
 * freed, so make sure to always allocate a new string
 */
-   reslen = strlen(in) + 1;
-   res = xmalloc(reslen);
-   res[0] = '\0';
+   res_len = strlen(src) + 1;
+   res = xmalloc(res_len);
+   out = res;
 
-   while ((src = strchr(in, '$'))) {
+   while ((in = strchr(src, '$'))) {
char *p, name[SYMBOL_MAXLENGTH];
-   const char *symval = "";
+   const char *sym_val = "";
struct symbol *sym;
-   size_t newlen;
+   size_t new_len, sym_len;
 
-   strncat(res, in, src - in);
-   src++;
+   strscpy(out, src, in - src);
+   out += in - src;
+   in++;
 
p = name;
-   while (isalnum(*src) || *src == '_')
-   *p++ = *src++;
+   while (isalnum(*in) || *in == '_')
+   *p++ = *in++;
*p = '\0';
 
sym = sym_find(name);
if (sym != NULL) {
sym_calc_value(sym);
-   symval = sym_get_string_value(sym);
+   sym_val = sym_get_string_value(sym);
}
 
-   newlen = strlen(res) + strlen(symval) + strlen(src) + 1;
-   if (newlen > reslen) {
-   reslen = newlen;
-   res = xrealloc(res, reslen);
+   sym_len = strlen(sym_val);
+   new_len = sym_len + strlen(res) + strlen(in) + 1;
+   if (new_len > res_len) {
+   res_len = new_len;
+   res = xrealloc(res, res_len);
}
 
-   strcat(res, symval);
-   in = src;
+   strscpy(out, sym_val, sym_len);
+   out += sym_len;
+   src = in;
}
-   strcat(res, in);
+   src_len = strlen(src);
+   strscpy(out, src, src_len);
+   out += src_len;
+   *out = '\0';
 
return res;
 }
 
-const char *sym_escape_string_value(const char *in)
+const char *sym_escape_string_value(const char *src)
 {
-   const char *p;
-   size_t reslen;
-   char *res;
-   size_t l;
+   const char *in;
+   size_t res_len, in_len;
+   char *res, *out;
 
-   reslen = strlen(in) + strlen("\"\"") + 1;
+   res_len = strlen(src) + strlen("\"\"") + 1;
 
-   p = in;
+   in = src;
for (;;) {
-   l = strcspn(p, "\"\\");
-   p += l;
+   in_len = strcspn(in, "\"\\");
+   in += in_len;
 
-   if (p[0] == '\0')
+   if (*in == '\0')
break;
 
-   reslen++;
-   p++;
+