diff mbox series

[RFC,v1,17/21] cli: hush_2021: Enable using \< and \> as string compare operators.

Message ID 20211231161327.24918-18-francis.laniel@amarulasolutions.com
State RFC
Delegated to: Tom Rini
Headers show
Series Modernize U-Boot shell | expand

Commit Message

Francis Laniel Dec. 31, 2021, 4:13 p.m. UTC
In Busybox hush, '<' and '>' are used as redirection operators.
For example, cat foo > bar will write content of file foo inside file bar.
In U-Boot, we do not have file system, so we can hardly redirect command output
inside a file.

But, in actual U-Boot hush, these operators ('<' and '>') are used as string
compare operators.
For example, test aaa < bbb returns 0 as aaa is before bbb in the dictionary.
Busybox hush also permits this, but operators need to be escaped ('\<' and
'\>'), so we stick here to this behavior.
Also, if escaping is needed it permits the developer to think about its code, as
in a lot of case, we want to compare integers (using '-lt' or '-gt') rather than
strings.

Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
---
 common/cli_hush_2021_upstream.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

Comments

Simon Glass Jan. 12, 2022, 8:03 p.m. UTC | #1
Hi Francis,

On Fri, 31 Dec 2021 at 09:14, Francis Laniel
<francis.laniel@amarulasolutions.com> wrote:
>
> In Busybox hush, '<' and '>' are used as redirection operators.
> For example, cat foo > bar will write content of file foo inside file bar.
> In U-Boot, we do not have file system, so we can hardly redirect command output
> inside a file.
>
> But, in actual U-Boot hush, these operators ('<' and '>') are used as string
> compare operators.
> For example, test aaa < bbb returns 0 as aaa is before bbb in the dictionary.
> Busybox hush also permits this, but operators need to be escaped ('\<' and
> '\>'), so we stick here to this behavior.
> Also, if escaping is needed it permits the developer to think about its code, as
> in a lot of case, we want to compare integers (using '-lt' or '-gt') rather than
> strings.
>
> Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> ---
>  common/cli_hush_2021_upstream.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

We should add a file interface to U-Boot. Is that the only impediment
to implementing redirection? I thought the problem was lack of fork()
?

Regards,
Simon
Francis Laniel Feb. 6, 2022, 6:23 p.m. UTC | #2
Le mercredi 12 janvier 2022, 21:03:40 CET Simon Glass a écrit :
> Hi Francis,
> 
> On Fri, 31 Dec 2021 at 09:14, Francis Laniel
> 
> <francis.laniel@amarulasolutions.com> wrote:
> > In Busybox hush, '<' and '>' are used as redirection operators.
> > For example, cat foo > bar will write content of file foo inside file bar.
> > In U-Boot, we do not have file system, so we can hardly redirect command
> > output inside a file.
> > 
> > But, in actual U-Boot hush, these operators ('<' and '>') are used as
> > string compare operators.
> > For example, test aaa < bbb returns 0 as aaa is before bbb in the
> > dictionary. Busybox hush also permits this, but operators need to be
> > escaped ('\<' and '\>'), so we stick here to this behavior.
> > Also, if escaping is needed it permits the developer to think about its
> > code, as in a lot of case, we want to compare integers (using '-lt' or
> > '-gt') rather than strings.
> > 
> > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> > ---
> > 
> >  common/cli_hush_2021_upstream.c | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> We should add a file interface to U-Boot. Is that the only impediment
> to implementing redirection? I thought the problem was lack of fork()
> ?

From my understanding of Busybox hush code dealing with I/O redirection 
(mainly parse_redirect() and struct redir_struct), it seems fork is never used 
here.
So, if we add something to emulate file descriptor it should be OK to add this 
feature to U-Boot.

> Regards,
> Simon
diff mbox series

Patch

diff --git a/common/cli_hush_2021_upstream.c b/common/cli_hush_2021_upstream.c
index f2619eb49a..d1f76d4df5 100644
--- a/common/cli_hush_2021_upstream.c
+++ b/common/cli_hush_2021_upstream.c
@@ -6128,7 +6128,33 @@  static struct pipe *parse_stream(char **pstring,
 			if (parse_redirect(&ctx, redir_fd, redir_style, input))
 				goto parse_error_exitcode1;
 			continue; /* get next char */
-#endif /* !__U_BOOT__ */
+#else /* __U_BOOT__ */
+			/*
+			 * In U-Boot, '<' and '>' can be used in test command to test if
+			 * a string is, alphabetically, before or after another.
+			 * In 2021 Busybox hush, using test foo < bar will show the
+			 * following error:
+			 * hush: can't open 'bar': No such file or directory
+			 * Indeed, in this context, '>' and '<' are used as redirection
+			 * operators, to use them as sorting operators, one should use:
+			 * [[ foo < bar ]]
+			 * Or:
+			 * test foo \< bar
+			 *
+			 * For the moment, we will not implement [[ ]], so we will need
+			 * the user to escape these operators to use them as string
+			 * comparaison operators.
+			 * NOTE In my opinion, when you use '<' or '>' I am almost sure
+			 * you wanted to use "-gt" or "-lt" in place, so thinking to
+			 * escape these will make you should check your code (sh syntax
+			 * at this level is, for me, error prone).
+			 */
+			case '>':
+				/* fallthrough */
+			case '<':
+				printf("Redirection operator ('%c') is not supported!\nPlease escape it ('\\%c') to use it as string comparaison operator.\n", (char) ch, (char) ch);
+				goto parse_error_exitcode1;
+#endif /* __U_BOOT__ */
 		case '#':
 			if (ctx.word.length == 0 && !ctx.word.has_quoted_part) {
 				/* skip "#comment" */