[PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE

[PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE

[Alan Ott]

print_menu_entry() in common/menu.c depeneds on process_escape_sequence().

Signed-off-by: Alan Ott <alan@signal11.us>
---
 common/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 73d620a..ebb9400 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -342,6 +342,7 @@ config AUTO_COMPLETE
 config MENU
 	bool
 	prompt "Menu Framework"
+	depends on PROCESS_ESCAPE_SEQUENCE
 	help
 	   a menu framework that allow us to create list menu to simplify
 	   barebox and make it more user-frendly
-- 
1.7.0.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE

[Sascha Hauer]

Hi Alan,

On Mon, May 21, 2012 at 11:53:36AM -0400, Alan Ott wrote:
> print_menu_entry() in common/menu.c depeneds on process_escape_sequence().
> 
> Signed-off-by: Alan Ott <alan@signal11.us>

Applied, thanks

Sascha

> ---
>  common/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 73d620a..ebb9400 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -342,6 +342,7 @@ config AUTO_COMPLETE
>  config MENU
>  	bool
>  	prompt "Menu Framework"
> +	depends on PROCESS_ESCAPE_SEQUENCE
>  	help
>  	   a menu framework that allow us to create list menu to simplify
>  	   barebox and make it more user-frendly
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:53 Mon 21 May     , Alan Ott wrote:
> print_menu_entry() in common/menu.c depeneds on process_escape_sequence().
> 
> Signed-off-by: Alan Ott <alan@signal11.us>
> ---
>  common/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 73d620a..ebb9400 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -342,6 +342,7 @@ config AUTO_COMPLETE
>  config MENU
>  	bool
>  	prompt "Menu Framework"
> +	depends on PROCESS_ESCAPE_SEQUENCE
>  	help
>  	   a menu framework that allow us to create list menu to simplify
>  	   barebox and make it more user-frendly
no the menu does not need this as I use it withouth hush

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH] common: Make HUSH_SHELL select PROCESS_ESCAPE_SEQUENCE

[Alan Ott]

Possibly among other things, print_menu_entry() in common/menu.c calls on
process_escape_sequence() when HUSH_SHELL is selected.

Signed-off-by: Alan Ott <alan@signal11.us>
---
 common/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 73d620a..7aa50b6 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -280,6 +280,7 @@ choice
 		select COMMAND_SUPPORT
 		select PARAMETER
 		select BINFMT
+		select PROCESS_ESCAPE_SEQUENCE
 		help
 		  Enable hush support. This is the most advanced shell available
 		  for barebox.
-- 
1.7.0.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE

[Alan Ott]

On 05/21/2012 06:44 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 11:53 Mon 21 May     , Alan Ott wrote:
>> print_menu_entry() in common/menu.c depeneds on process_escape_sequence().
>>
>> Signed-off-by: Alan Ott <alan@signal11.us>
>> ---
>>  common/Kconfig |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 73d620a..ebb9400 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -342,6 +342,7 @@ config AUTO_COMPLETE
>>  config MENU
>>  	bool
>>  	prompt "Menu Framework"
>> +	depends on PROCESS_ESCAPE_SEQUENCE
>>  	help
>>  	   a menu framework that allow us to create list menu to simplify
>>  	   barebox and make it more user-frendly
> no the menu does not need this as I use it withouth hush
>

Jean-Christophe,

You're right. I just posted another patch which may work better.

Thanks!

Alan.


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE

[Jean-Christophe PLAGNIOL-VILLARD]

Hi

Please revert break my work on at91bootstrap where we use the menu without any shell support

And the does work with or without hush support

Best Regards,
J.

On May 22, 2012, at 4:06 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi Alan,
> 
> On Mon, May 21, 2012 at 11:53:36AM -0400, Alan Ott wrote:
>> print_menu_entry() in common/menu.c depeneds on process_escape_sequence().
>> 
>> Signed-off-by: Alan Ott <alan@signal11.us>
> 
> Applied, thanks
> 
> Sascha
> 
>> ---
>> common/Kconfig |    1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>> 
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 73d620a..ebb9400 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -342,6 +342,7 @@ config AUTO_COMPLETE
>> config MENU
>>    bool
>>    prompt "Menu Framework"
>> +    depends on PROCESS_ESCAPE_SEQUENCE
>>    help
>>       a menu framework that allow us to create list menu to simplify
>>       barebox and make it more user-frendly
>> -- 
>> 1.7.0.4
>> 
>> 
>> _______________________________________________
>> barebox mailing list
>> barebox@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/barebox
>> 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected

[Alan Ott]

print_menu_entry() in common/menu.c calls on process_escape_sequence() when
HUSH_SHELL is selected.

Signed-off-by: Alan Ott <alan@signal11.us>
---
 common/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 73d620a..7cd0666 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -342,6 +342,7 @@ config AUTO_COMPLETE
 config MENU
 	bool
 	prompt "Menu Framework"
+	select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH
 	help
 	   a menu framework that allow us to create list menu to simplify
 	   barebox and make it more user-frendly
-- 
1.7.0.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] common: Make MENU depend on PROCESS_ESCAPE_SEQUENCE

[Alan Ott]

On 05/21/2012 10:40 PM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Hi
>
> Please revert break my work on at91bootstrap where we use the menu without any shell support
>
> And the does work with or without hush support

Upon closer inspection, it should be:

     config MENU
     bool
     prompt "Menu Framework"
+    select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH

I have posted v2 which does this, and I agree that the original patch
should be reverted in favor of the new one.

Steps to reproduce on the master branch:

1. build on amd64 (no ARCH= or CROSS_COMPILE= or anything else)
2. make distclean
3. make menuconfig
4.    Select "Menu Framework" from General Settings
5.    leave everything else default.
6. make
7. output at [1] (link error).

Alan.

[1]
  LD      barebox
common/built-in.o: In function `print_menu_entry':
menu.c:(.text+0x5415): undefined reference to `process_escape_sequence'
common/built-in.o: In function `print_menu':
menu.c:(.text+0x54a3): undefined reference to `process_escape_sequence'
collect2: ld returned 1 exit status
make: *** [barebox] Error 1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected

[Jean-Christophe PLAGNIOL-VILLARD]

On 23:32 Mon 21 May     , Alan Ott wrote:
> print_menu_entry() in common/menu.c calls on process_escape_sequence() when
> HUSH_SHELL is selected.
> 
> Signed-off-by: Alan Ott <alan@signal11.us>
> ---
>  common/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 73d620a..7cd0666 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -342,6 +342,7 @@ config AUTO_COMPLETE
>  config MENU
>  	bool
>  	prompt "Menu Framework"
> +	select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH
>  	help
>  	   a menu framework that allow us to create list menu to simplify
>  	   barebox and make it more user-frendly
you miss the point of the code

the PROCESS_ESCAPE_SEQUENCE is optionnal and need to stay optionnal
evenif Hush is enable

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected

[Alan Ott]

On 05/22/2012 05:27 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 23:32 Mon 21 May     , Alan Ott wrote:
>> print_menu_entry() in common/menu.c calls on process_escape_sequence() when
>> HUSH_SHELL is selected.
>>
>> Signed-off-by: Alan Ott <alan@signal11.us>
>> ---
>>  common/Kconfig |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 73d620a..7cd0666 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -342,6 +342,7 @@ config AUTO_COMPLETE
>>  config MENU
>>  	bool
>>  	prompt "Menu Framework"
>> +	select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH
>>  	help
>>  	   a menu framework that allow us to create list menu to simplify
>>  	   barebox and make it more user-frendly
> you miss the point of the code
>
> the PROCESS_ESCAPE_SEQUENCE is optionnal and need to stay optionnal
> evenif Hush is enable

It's required if both HUSH_SHELL _and_ MENU are selected. The code in
common/menu.c looks like this:

                if (IS_ENABLED(CONFIG_SHELL_HUSH)) {
                        process_escape_sequence(m->display,
m->display_buffer,
                                                m->display_buffer_size);
                        puts(m->display_buffer);
                } else {
                        puts(m->display);
                }

Thus, the configuration needs to say, if you turn on MENU (enabling
menu.c), then select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is also
selected (because it will try to call process_escape_sequence()).

As I described in my other email, it's very easy to create a test case
which does not build.

Keep in mind, I'm talking about making a Kconfig which is correct for
the code as the code currently is. Maybe in the opinion of some (you?),
it is the code that should be different, not the configuration. If you
or someone else has a better idea how to fix this, that would be fine
too. Right now however, it is broken.

Alan.


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2] common: Make MENU select PROCESS_ESCAPE_SEQUENCE if HUSH_SHELL is selected

[Sascha Hauer]

On Tue, May 22, 2012 at 11:08:51AM -0400, Alan Ott wrote:
> On 05/22/2012 05:27 AM, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 23:32 Mon 21 May     , Alan Ott wrote:
> >> print_menu_entry() in common/menu.c calls on process_escape_sequence() when
> >> HUSH_SHELL is selected.
> >>
> >> Signed-off-by: Alan Ott <alan@signal11.us>
> >> ---
> >>  common/Kconfig |    1 +
> >>  1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/common/Kconfig b/common/Kconfig
> >> index 73d620a..7cd0666 100644
> >> --- a/common/Kconfig
> >> +++ b/common/Kconfig
> >> @@ -342,6 +342,7 @@ config AUTO_COMPLETE
> >>  config MENU
> >>  	bool
> >>  	prompt "Menu Framework"
> >> +	select PROCESS_ESCAPE_SEQUENCE if SHELL_HUSH
> >>  	help
> >>  	   a menu framework that allow us to create list menu to simplify
> >>  	   barebox and make it more user-frendly
> > you miss the point of the code
> >
> > the PROCESS_ESCAPE_SEQUENCE is optionnal and need to stay optionnal
> > evenif Hush is enable
> 
> It's required if both HUSH_SHELL _and_ MENU are selected. The code in
> common/menu.c looks like this:
> 
>                 if (IS_ENABLED(CONFIG_SHELL_HUSH)) {
>                         process_escape_sequence(m->display,
> m->display_buffer,
>                                                 m->display_buffer_size);
>                         puts(m->display_buffer);
>                 } else {
>                         puts(m->display);
>                 }

From what I see your v2 patch is correct. Another possibility would
maybe be replacing the if above with
IS_ENABLED(CONFIG_PROCESS_ESCAPE_SEQUENCE), but I don't know if that
would produce correct code.

Jean-Christophe?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox