From: "Yann E. MORIN" <[email protected]>
Hello Michal,
Please pull these post-rc1 kconfig cleanups, all after
review and comments by Jean:
- simplify and cleanup the symbol-search code
- better documentation about the symbols ordering in search results
- eye-candy in the title of the search-box in [mn]conf
Regards,
Yann E. MORIN.
The following changes since commit b57caaaed2bd127fe656e6c145970ed6a05c0125:
kconfig: allow "hex" and "range" to support longs (2013-06-29 15:30:17 +0200)
are available in the git repository at:
git://gitorious.org/linux-kconfig/linux-kconfig.git yem-kconfig-rc-fixes
for you to fetch changes up to 508382a0428f2b2f49da0e0e89c921f07c9306aa:
kconfig: simplify symbol-search code (2013-07-16 20:39:42 +0200)
----------------------------------------------------------------
Yann E. MORIN (6):
Documentation/kconfig: more concise and straightforward search explanation
kconfig: avoid multiple calls to strlen
kconfig/[mn]conf: shorten title in search-box
kconfig: minor style fixes in symbol-search code
kconfig: don't allocate n+1 elements in temporary array
kconfig: simplify symbol-search code
Documentation/kbuild/kconfig.txt | 8 +++----
scripts/kconfig/mconf.c | 4 ++--
scripts/kconfig/nconf.c | 4 ++--
scripts/kconfig/symbol.c | 49 ++++++++++++++++------------------------
4 files changed, 27 insertions(+), 38 deletions(-)
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
From: "Yann E. MORIN" <[email protected]>
There is no need for a double indirection in the temporary array that
stores the internediate search results.
Reported-by: Jean Delvare <[email protected]>
Signed-off-by: "Yann E. MORIN" <[email protected]>
Reviewed-by: Jean Delvare <[email protected]>
---
scripts/kconfig/symbol.c | 30 +++++++++++-------------------
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 08d4401..a76b8fd 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -965,8 +965,8 @@ struct sym_match {
*/
static int sym_rel_comp(const void *sym1, const void *sym2)
{
- struct sym_match *s1 = *(struct sym_match **)sym1;
- struct sym_match *s2 = *(struct sym_match **)sym2;
+ const struct sym_match *s1 = sym1;
+ const struct sym_match *s2 = sym2;
int exact1, exact2;
/* Exact match:
@@ -992,7 +992,7 @@ static int sym_rel_comp(const void *sym1, const void *sym2)
struct symbol **sym_re_search(const char *pattern)
{
struct symbol *sym, **sym_arr = NULL;
- struct sym_match **sym_match_arr = NULL;
+ struct sym_match *sym_match_arr = NULL;
int i, cnt, size;
regex_t re;
regmatch_t match[1];
@@ -1005,7 +1005,6 @@ struct symbol **sym_re_search(const char *pattern)
return NULL;
for_all_symbols(i, sym) {
- struct sym_match *tmp_sym_match;
if (sym->flags & SYMBOL_CONST || !sym->name)
continue;
if (regexec(&re, sym->name, 1, match, 0))
@@ -1013,38 +1012,31 @@ struct symbol **sym_re_search(const char *pattern)
if (cnt >= size) {
void *tmp;
size += 16;
- tmp = realloc(sym_match_arr, size * sizeof(struct sym_match *));
+ tmp = realloc(sym_match_arr, size * sizeof(struct sym_match));
if (!tmp)
goto sym_re_search_free;
sym_match_arr = tmp;
}
sym_calc_value(sym);
- tmp_sym_match = (struct sym_match*)malloc(sizeof(struct sym_match));
- if (!tmp_sym_match)
- goto sym_re_search_free;
- tmp_sym_match->sym = sym;
/* As regexec returned 0, we know we have a match, so
* we can use match[0].rm_[se]o without further checks
*/
- tmp_sym_match->so = match[0].rm_so;
- tmp_sym_match->eo = match[0].rm_eo;
- sym_match_arr[cnt++] = tmp_sym_match;
+ sym_match_arr[cnt].so = match[0].rm_so;
+ sym_match_arr[cnt].eo = match[0].rm_eo;
+ sym_match_arr[cnt++].sym = sym;
}
if (sym_match_arr) {
- qsort(sym_match_arr, cnt, sizeof(struct sym_match*), sym_rel_comp);
+ qsort(sym_match_arr, cnt, sizeof(struct sym_match), sym_rel_comp);
sym_arr = malloc((cnt+1) * sizeof(struct symbol));
if (!sym_arr)
goto sym_re_search_free;
for (i = 0; i < cnt; i++)
- sym_arr[i] = sym_match_arr[i]->sym;
+ sym_arr[i] = sym_match_arr[i].sym;
sym_arr[cnt] = NULL;
}
sym_re_search_free:
- if (sym_match_arr) {
- for (i = 0; i < cnt; i++)
- free(sym_match_arr[i]);
- free(sym_match_arr);
- }
+ /* sym_match_arr can be NULL if no match, but free(NULL) is OK */
+ free(sym_match_arr);
regfree(&re);
return sym_arr;
--
1.8.1.2
From: "Yann E. MORIN" <[email protected]>
Two minor style fixes:
- no space before/after parenthesis in function definition
- no {} for single-line if()
And one grammar fix in a comment.
Reported-by: Jean Delvare <[email protected]>
Signed-off-by: "Yann E. MORIN" <[email protected]>
Cc: Jean Delvare <[email protected]>
Reviewed-by: Jean Delvare <[email protected]>
---
scripts/kconfig/symbol.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 020a0ac..b664d6e 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -963,7 +963,7 @@ struct sym_match {
* - first, symbols that match exactly
* - then, alphabetical sort
*/
-static int sym_rel_comp( const void *sym1, const void *sym2 )
+static int sym_rel_comp(const void *sym1, const void *sym2)
{
struct sym_match *s1 = *(struct sym_match **)sym1;
struct sym_match *s2 = *(struct sym_match **)sym2;
@@ -1014,9 +1014,8 @@ struct symbol **sym_re_search(const char *pattern)
void *tmp;
size += 16;
tmp = realloc(sym_match_arr, size * sizeof(struct sym_match *));
- if (!tmp) {
+ if (!tmp)
goto sym_re_search_free;
- }
sym_match_arr = tmp;
}
sym_calc_value(sym);
@@ -1024,7 +1023,7 @@ struct symbol **sym_re_search(const char *pattern)
if (!tmp_sym_match)
goto sym_re_search_free;
tmp_sym_match->sym = sym;
- /* As regexec return 0, we know we have a match, so
+ /* As regexec returned 0, we know we have a match, so
* we can use match[0].rm_[se]o without further checks
*/
tmp_sym_match->so = match[0].rm_so;
--
1.8.1.2
From: "Yann E. MORIN" <[email protected]>
Calls to strlen are costly, so avoid calling strln as much as we can.
Reported-by: Jean Delvare <[email protected]>
Signed-off-by: "Yann E. MORIN" <[email protected]>
Cc: Jean Delvare <[email protected]>
Reviewed-by: Jean Delvare <[email protected]>
---
scripts/kconfig/symbol.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index d550300..020a0ac 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -967,7 +967,7 @@ static int sym_rel_comp( const void *sym1, const void *sym2 )
{
struct sym_match *s1 = *(struct sym_match **)sym1;
struct sym_match *s2 = *(struct sym_match **)sym2;
- int l1, l2;
+ int exact1, exact2;
/* Exact match:
* - if matched length on symbol s1 is the length of that symbol,
@@ -978,11 +978,11 @@ static int sym_rel_comp( const void *sym1, const void *sym2 )
* exactly; if this is the case, we can't decide which comes first,
* and we fallback to sorting alphabetically.
*/
- l1 = s1->eo - s1->so;
- l2 = s2->eo - s2->so;
- if (l1 == strlen(s1->sym->name) && l2 != strlen(s2->sym->name))
+ exact1 = (s1->eo - s1->so) == strlen(s1->sym->name);
+ exact2 = (s2->eo - s2->so) == strlen(s2->sym->name);
+ if (exact1 && !exact2)
return -1;
- if (l1 != strlen(s1->sym->name) && l2 == strlen(s2->sym->name))
+ if (!exact1 && exact2)
return 1;
/* As a fallback, sort symbols alphabetically */
--
1.8.1.2
From: "Yann E. MORIN" <[email protected]>
The temporary array that stores the search results is not NULL-terminated,
so there is no reason to allocate n+1 elements.
Reported-by: Jean Delvare <[email protected]>
Signed-off-by: "Yann E. MORIN" <[email protected]>
Reviewed-by: Jean Delvare <[email protected]>
---
scripts/kconfig/symbol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index b664d6e..08d4401 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -1010,7 +1010,7 @@ struct symbol **sym_re_search(const char *pattern)
continue;
if (regexec(&re, sym->name, 1, match, 0))
continue;
- if (cnt + 1 >= size) {
+ if (cnt >= size) {
void *tmp;
size += 16;
tmp = realloc(sym_match_arr, size * sizeof(struct sym_match *));
--
1.8.1.2
From: "Yann E. MORIN" <[email protected]>
No need to repeat the 'CONFIG_' string in the title,
once is explicit enough.
Reported-by: Jean Delvare <[email protected]>
Signed-off-by: "Yann E. MORIN" <[email protected]>
Cc: Jean Delvare <[email protected]>
---
scripts/kconfig/mconf.c | 4 ++--
scripts/kconfig/nconf.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
index 18d3dc9..b20e851 100644
--- a/scripts/kconfig/mconf.c
+++ b/scripts/kconfig/mconf.c
@@ -401,8 +401,8 @@ static void search_conf(void)
struct subtitle_part stpart;
title = str_new();
- str_printf( &title, _("Enter %s (sub)string or regexp to search for "
- "(with or without \"%s\")"), CONFIG_, CONFIG_);
+ str_printf( &title, _("Enter (sub)string or regexp to search for "
+ "(with or without \"%s\")"), CONFIG_);
again:
dialog_clear();
diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
index 7975d8d..4fbecd2 100644
--- a/scripts/kconfig/nconf.c
+++ b/scripts/kconfig/nconf.c
@@ -695,8 +695,8 @@ static void search_conf(void)
int dres;
title = str_new();
- str_printf( &title, _("Enter %s (sub)string or regexp to search for "
- "(with or without \"%s\")"), CONFIG_, CONFIG_);
+ str_printf( &title, _("Enter (sub)string or regexp to search for "
+ "(with or without \"%s\")"), CONFIG_);
again:
dres = dialog_inputbox(main_window,
--
1.8.1.2
From: "Yann E. MORIN" <[email protected]>
Re-phrase the explanations on the sorting of search results, in a more
concise and complete way.
Drop reference to the user's locale when sorting alphabetically, since
this is implicit.
Reported-by: Jean Delvare <[email protected]>
Signed-off-by: "Yann E. MORIN" <[email protected]>
Cc: Jean Delvare <[email protected]>
Reviewed-by: Jean Delvare <[email protected]>
---
Documentation/kbuild/kconfig.txt | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/Documentation/kbuild/kconfig.txt b/Documentation/kbuild/kconfig.txt
index e9f9e76..d58a2ea 100644
--- a/Documentation/kbuild/kconfig.txt
+++ b/Documentation/kbuild/kconfig.txt
@@ -175,11 +175,9 @@ Searching in menuconfig:
/^hotplug
When searching, symbols are sorted thus:
- - exact match first: an exact match is when the search matches
- the complete symbol name;
- - alphabetical order: when two symbols do not match exactly,
- they are sorted in alphabetical order (in the user's current
- locale).
+ - first, exact matches, sorted alphabetically (an exact match
+ is when the search matches the complete symbol name);
+ - then, other matches, sorted alphabetically.
For example: ^ATH.K matches:
ATH5K ATH9K ATH5K_AHB ATH5K_DEBUG [...] ATH6KL ATH6KL_DEBUG
[...] ATH9K_AHB ATH9K_BTCOEX_SUPPORT ATH9K_COMMON [...]
--
1.8.1.2
Le Tuesday 16 July 2013 à 22:39 +0200, Yann E. MORIN a écrit :
> From: "Yann E. MORIN" <[email protected]>
>
> No need to repeat the 'CONFIG_' string in the title,
> once is explicit enough.
>
> Reported-by: Jean Delvare <[email protected]>
> Signed-off-by: "Yann E. MORIN" <[email protected]>
> Cc: Jean Delvare <[email protected]>
> ---
> scripts/kconfig/mconf.c | 4 ++--
> scripts/kconfig/nconf.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
> index 18d3dc9..b20e851 100644
> --- a/scripts/kconfig/mconf.c
> +++ b/scripts/kconfig/mconf.c
> @@ -401,8 +401,8 @@ static void search_conf(void)
> struct subtitle_part stpart;
>
> title = str_new();
> - str_printf( &title, _("Enter %s (sub)string or regexp to search for "
> - "(with or without \"%s\")"), CONFIG_, CONFIG_);
> + str_printf( &title, _("Enter (sub)string or regexp to search for "
> + "(with or without \"%s\")"), CONFIG_);
>
> again:
> dialog_clear();
> diff --git a/scripts/kconfig/nconf.c b/scripts/kconfig/nconf.c
> index 7975d8d..4fbecd2 100644
> --- a/scripts/kconfig/nconf.c
> +++ b/scripts/kconfig/nconf.c
> @@ -695,8 +695,8 @@ static void search_conf(void)
> int dres;
>
> title = str_new();
> - str_printf( &title, _("Enter %s (sub)string or regexp to search for "
> - "(with or without \"%s\")"), CONFIG_, CONFIG_);
> + str_printf( &title, _("Enter (sub)string or regexp to search for "
> + "(with or without \"%s\")"), CONFIG_);
>
> again:
> dres = dialog_inputbox(main_window,
Reviewed-by: Jean Delvare <[email protected]>
--
Jean Delvare
Suse L3
Le Tuesday 16 July 2013 à 22:39 +0200, Yann E. MORIN a écrit :
> From: "Yann E. MORIN" <[email protected]>
>
> Hello Michal,
>
> Please pull these post-rc1 kconfig cleanups, all after
> review and comments by Jean:
> - simplify and cleanup the symbol-search code
> - better documentation about the symbols ordering in search results
> - eye-candy in the title of the search-box in [mn]conf
>
> Regards,
> Yann E. MORIN.
It looks very good to me now, thanks Yann for the good work!
--
Jean Delvare
Suse L3
On 17.7.2013 11:59, Jean Delvare wrote:
> Le Tuesday 16 July 2013 à 22:39 +0200, Yann E. MORIN a écrit :
>> From: "Yann E. MORIN" <[email protected]>
>>
>> Hello Michal,
>>
>> Please pull these post-rc1 kconfig cleanups, all after
>> review and comments by Jean:
>> - simplify and cleanup the symbol-search code
>> - better documentation about the symbols ordering in search results
>> - eye-candy in the title of the search-box in [mn]conf
>>
>> Regards,
>> Yann E. MORIN.
>
> It looks very good to me now, thanks Yann for the good work!
It does look good indeed. But I am wondering if it's necessary for 3.11.
AFAICS the only user-visible changes are
[PATCH 1/6] Documentation/kconfig: more concise and straightforward
search explanation
[PATCH 3/6] kconfig/[mn]conf: shorten title in search-box
and the rest are code cleanups (however worthwhile), is that correct?
There is no change in the search behavior, is there? Then I would say
merge this for 3.12-rc1. Maybe merge the documentation patch for 3.11.
Thanks,
Michal
Michal, All,
On Tuesday 23 July 2013 15:37:37 Michal Marek wrote:
> On 17.7.2013 11:59, Jean Delvare wrote:
> > Le Tuesday 16 July 2013 à 22:39 +0200, Yann E. MORIN a écrit :
> >> From: "Yann E. MORIN" <[email protected]>
> >>
> >> Hello Michal,
> >>
> >> Please pull these post-rc1 kconfig cleanups, all after
> >> review and comments by Jean:
> >> - simplify and cleanup the symbol-search code
> >> - better documentation about the symbols ordering in search results
> >> - eye-candy in the title of the search-box in [mn]conf
> >>
> >> Regards,
> >> Yann E. MORIN.
> >
> > It looks very good to me now, thanks Yann for the good work!
>
> It does look good indeed. But I am wondering if it's necessary for 3.11.
> AFAICS the only user-visible changes are
>
> [PATCH 1/6] Documentation/kconfig: more concise and straightforward
> search explanation
> [PATCH 3/6] kconfig/[mn]conf: shorten title in search-box
>
> and the rest are code cleanups (however worthwhile), is that correct?
> There is no change in the search behavior, is there? Then I would say
> merge this for 3.12-rc1. Maybe merge the documentation patch for 3.11.
I think you can merge all for 3.12-rc1, in fact.
Even patches 1 and 3 are only eye-candy, and can well wait for 3.12.
Not sure why I wanted them in an -rc in the first place, so I'm fine
with all 6 patches going in for 3.12.
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +0/33 662376056 | Software Designer | \ / CAMPAIGN | ^ |
| --==< O_o >==-- '------------.-------: X AGAINST | /e\ There is no |
| http://ymorin.is-a-geek.org/ | (*_*) | / \ HTML MAIL | """ conspiracy. |
'------------------------------'-------'------------------'--------------------'
Yann E. MORIN wrote:
> Michal, All,
>
> On Tuesday 23 July 2013 15:37:37 Michal Marek wrote:
>> It does look good indeed. But I am wondering if it's necessary for 3.11.
>> AFAICS the only user-visible changes are
>>
>> [PATCH 1/6] Documentation/kconfig: more concise and straightforward
>> search explanation
>> [PATCH 3/6] kconfig/[mn]conf: shorten title in search-box
>>
>> and the rest are code cleanups (however worthwhile), is that correct?
>> There is no change in the search behavior, is there? Then I would say
>> merge this for 3.12-rc1. Maybe merge the documentation patch for 3.11.
>
> I think you can merge all for 3.12-rc1, in fact.
> Even patches 1 and 3 are only eye-candy, and can well wait for 3.12.
OK, I merged it to kbuild.git#kconfig then. It will show up a bit later
on git.kernel.org, as I forgot my laptop with the ssh key at home :).
Michal