public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: Sean Anderson <seanga2@gmail.com>
Cc: "Steve Bennett" <steveb@workware.net.au>,
	"Rasmus Villemoes" <rasmus.villemoes@prevas.dk>,
	u-boot@lists.denx.de, "Tom Rini" <trini@konsulko.com>,
	"Marek Behún" <marek.behun@nic.cz>,
	"Simon Glass" <sjg@chromium.org>,
	"Roland Gaudig" <roland.gaudig-oss@weidmueller.com>,
	"Heinrich Schuchardt" <xypron.glpk@gmx.de>,
	"Kostas Michalopoulos" <badsector@runtimeterror.com>
Subject: Re: [RFC PATCH 03/28] cli: lil: Replace strclone with strdup
Date: Mon, 05 Jul 2021 19:50:34 +0200	[thread overview]
Message-ID: <163090.1625507434@gemini.denx.de> (raw)
In-Reply-To: <5a967151-94f0-6037-2d02-0114c43b846c@gmail.com>

Dear Sean,

In message <5a967151-94f0-6037-2d02-0114c43b846c@gmail.com> you wrote:
>
> AIUI hush has diverged significantly from what U-Boot has. This would
> not be an "update" moreso than a complete port in the style of the
> current series.

Agreed.  However, as you write this it sounds like a big problem
only.  I disagree here - it is also a chance to do a few things
different than with the original port.

Please keep in mind when this original port of the hush shell was
done:  it was a time when many systems came with a total of 4 MB
flash memory, and not only U-Boot, but also the Linux kernel and a
ram-disk image or such had to fit into that.  At that time 40 or 60 kB
code size for just a fancy shell was immense!

Under such restrictions (and the need to complete the task with a
given budget) many things were just commented out which from today's
point of view would be nice to have.


This is not a problem of the code complexity or resources, but only
of different requirements and different resources.


> I don't think sh-style shells are a good match for U-Boot's execution
> environment in the first place. The fundamental idea of an sh-style
> shell is that the output of one command can be redirected to the input
> (or arguments) of another command. This cannot be done (or rather would
> be difficult to do) in U-Boot for a few reasons

There is an old saying:

	The loser says: "It might be possible, but it is too difficult."
	The winner says: "It might be difficult, but it is possible."

Apparently you take another position here than me.

First, I disagree that pipes are the "fundamental idea" of a shell.
It is a fundamental idea of the UNIX operating systems, indeed.

But please keep in mind that we are here in a boot loader, not in a
full-blown OS context.

> * U-Boot does not support multithreading. Existing shells tend to depend
>    strongly on this feature of the enviromnent. Many of the changes to
>    U-Boot's hush are solely to deal with the lack of this feature.

I disagree to both parts of your statement.

Multithreading (or rather, a concept of separate processes which can
eventually even run in parallel) is very nice to have, but it is not
mandatory in most cases.  Command pipes can almost always be
strictly sequentialized and thus run in a single-task environment,
too.  I'm not sure, but I think I even remember pipes in the "shell"
of the CPM "OS" on Z80 processors a number of decades ago...

And the fact that our current version of hush did not attempt to
keep these features was much more driven by strict memory footprint
limitations that anything else.  I claim it would have been
possible, and still is.

You also don't mention (and I guess you oversee) another critical
fact: so far, U-Boot has no concept of files.  The classic design
principle "everything is a file" is missing even more than the
concept of processes.

> * Existing commands do not read from stdin, nor do they print useful
>    information to stdout. Command output is designed for human
>    consumption and is substantially more verbose than typical unix
>    commands.

This is a statement which is correct, but it does not contain any
pro or con for the discussion here.

And if we had the features, it would be easy to fix.

> * Tools such as grep, cut, tr, sed, sort, uniq, etc. which are extremely
>    useful when working with streams are not present in U-Boot.

You are talking about OS environments here.  But we are a boot
loader.

Also, this argument is not fair, as the suggested LIL does not
provide grep, sed, awk, sort, uniq etc. functionality either, or
does it?

> And of course, this feature is currently not present in U-Boot. To get

Correct, and for very good reasons.

> which will set my_uuid to the uuid of the selected partition. My issue
> with this is threefold: every command must add new syntax to do this,
> that syntax is inconsistent, and it prevents easy composition. Consider
> a script which wants to iterate over partitions. Instead of doing
>
> 	for p in $(part list mmc 0); do
> 		# ...
> 	done
>
> it must instead do
>
> 	part list mmc 0 partitions
> 	for p in $partitions; do
> 		# ...
> 	done
>
> which unnecessarily adds an extra step. This overhead accumulates with
> each command which adds something like this.

Apparently you fail to understand that this "extra step" is primarily
due to the fact that we are single tasking. Even if we has command
substitution (like we could have with hush) the execution sequence
would be serialized in exactly the same way under the hood - it's
just not as directly visible.

> Both of these workarounds are natural consequences of using a sh-tyle
> shell in an environment it is not suited for. If we are going to go to

No, they are not.  They are much more the consequence of no strict
design guidelines for commands, so everybody implements what fits his
purposes best.  A cleaner approach does not require any of the
additional features you've asked for - it would be possible as is.

> the effort of porting a new language (which must be done no matter if
> we use Hush or some other language), we should pick one which has better
> support for single-threaded programming.

In which way do you think "a new language" needs to be ported when
switching from an old to a new version of hush?  It would be still a
(mostly) POSIX compatible shell, with some restrictions.


I can fully understand that you defend your proposal, but let's be
fair and stick with the facts.  Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
To get something done, a committee should consist  of  no  more  than
three men, two of them absent.


  parent reply	other threads:[~2021-07-05 17:50 UTC|newest]

Thread overview: 107+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01  6:15 [RFC PATCH 00/28] cli: Add a new shell Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 01/28] Add Zlib License Sean Anderson
2021-07-05 15:29   ` Simon Glass
2021-07-01  6:15 ` [RFC PATCH 02/28] cli: Add LIL shell Sean Anderson
2021-07-02 11:03   ` Wolfgang Denk
2021-07-02 13:33     ` Sean Anderson
2021-07-03  2:12       ` Sean Anderson
2021-07-03 19:33         ` Wolfgang Denk
2021-07-05 15:29           ` Simon Glass
2021-07-05 19:10           ` Tom Rini
2021-07-05 19:47             ` Sean Anderson
2021-07-05 19:53               ` Tom Rini
2021-07-05 19:55                 ` Sean Anderson
2021-07-06  7:46               ` Wolfgang Denk
2021-07-06  7:52                 ` Michael Nazzareno Trimarchi
2021-07-06 14:57                   ` Simon Glass
2021-07-06 15:48                     ` Tom Rini
2021-07-07  8:22                     ` Michael Nazzareno Trimarchi
2021-07-06 14:54                 ` Tom Rini
2021-07-07  8:15                   ` Wolfgang Denk
2021-07-07 13:58                     ` Tom Rini
2021-07-07 14:10                       ` Wolfgang Denk
2021-07-07 14:14                         ` Tom Rini
2021-07-07 14:23                           ` Wolfgang Denk
2021-07-06  7:44             ` Wolfgang Denk
2021-07-06 15:43               ` Tom Rini
2021-07-06 16:09                 ` Kostas Michalopoulos
2021-07-07 13:32                   ` Sean Anderson
2021-07-07  8:15                 ` Wolfgang Denk
2021-07-07 13:46                   ` Sean Anderson
2021-07-07 13:51                     ` Tom Rini
2021-07-07 13:58                   ` Tom Rini
2021-07-07 14:48                   ` Marek Behun
2021-07-08  5:19                     ` Michael Nazzareno Trimarchi
2021-07-08 15:33                       ` Tom Rini
2021-07-08  4:56               ` Sean Anderson
2021-07-08 17:00                 ` Wolfgang Denk
2021-07-03 19:23       ` Wolfgang Denk
2021-07-01  6:15 ` [RFC PATCH 03/28] cli: lil: Replace strclone with strdup Sean Anderson
2021-07-02  8:36   ` Rasmus Villemoes
2021-07-02 11:38     ` Wolfgang Denk
2021-07-02 13:38     ` Sean Anderson
2021-07-02 14:28       ` Tom Rini
2021-07-02 22:18       ` Kostas Michalopoulos
2021-07-03  2:28         ` Sean Anderson
2021-07-03 19:26       ` Wolfgang Denk
2021-07-05  5:07         ` Steve Bennett
2021-07-05 14:42           ` Sean Anderson
2021-07-05 15:29             ` Simon Glass
2021-07-05 15:42               ` Sean Anderson
2021-07-05 17:50             ` Wolfgang Denk [this message]
2021-07-08  4:37               ` Sean Anderson
2021-07-08 16:13                 ` Wolfgang Denk
2021-07-01  6:15 ` [RFC PATCH 04/28] cli: lil: Remove most functions by default Sean Anderson
2021-07-05 15:29   ` Simon Glass
2021-07-01  6:15 ` [RFC PATCH 05/28] cli: lil: Rename some functions to be more like TCL Sean Anderson
2021-07-05 15:29   ` Simon Glass
2021-07-05 15:54     ` Sean Anderson
2021-07-05 17:58       ` Wolfgang Denk
2021-07-05 18:51         ` Tom Rini
2021-07-05 21:02           ` Simon Glass
2021-07-05 21:36             ` Tom Rini
2021-07-06  7:52           ` Wolfgang Denk
2021-07-06 15:21             ` Simon Glass
2021-07-06 15:33             ` Tom Rini
2021-07-06 16:00               ` Kostas Michalopoulos
2021-07-07  8:16               ` Wolfgang Denk
2021-07-07 13:58                 ` Tom Rini
2021-07-05 19:46         ` Sean Anderson
2021-07-06  7:50           ` Wolfgang Denk
2021-07-08  4:47             ` Sean Anderson
2021-07-08 16:21               ` Wolfgang Denk
2021-07-01  6:15 ` [RFC PATCH 06/28] cli: lil: Convert some defines to enums Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 07/28] cli: lil: Simplify callbacks Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 08/28] cli: lil: Handle commands with dots Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 09/28] cli: lil: Use error codes Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 10/28] cli: lil: Add printf-style format helper for errors Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 11/28] cli: lil: Add several helper functions " Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 12/28] cli: lil: Check for ctrl-c Sean Anderson
2021-07-05 15:29   ` Simon Glass
2021-07-01  6:15 ` [RFC PATCH 13/28] cli: lil: Wire up LIL to the rest of U-Boot Sean Anderson
2021-07-02  8:18   ` Rasmus Villemoes
2021-07-02 13:40     ` Sean Anderson
2021-07-05 15:29   ` Simon Glass
2021-07-01  6:15 ` [RFC PATCH 14/28] cli: lil: Document structures Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 15/28] cli: lil: Convert LIL_ENABLE_POOLS to Kconfig Sean Anderson
2021-07-01  6:15 ` [RFC PATCH 16/28] cli: lil: Convert LIL_ENABLE_RECLIMIT to KConfig Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 17/28] test: Add tests for LIL Sean Anderson
2021-07-05 15:29   ` Simon Glass
2021-07-01  6:16 ` [RFC PATCH 18/28] cli: lil: Remove duplicate function bodies Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 19/28] cli: lil: Add "symbol" structure Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 20/28] cli: lil: Add config to enable debug output Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 21/28] cli: lil: Add a distinct parsing step Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 22/28] env: Add a priv pointer to hwalk_r Sean Anderson
2021-07-01 20:10   ` Tom Rini
2021-07-01  6:16 ` [RFC PATCH 23/28] cli: lil: Handle OOM for hm_put Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 24/28] cli: lil: Make proc always take 3 arguments Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 25/28] cli: lil: Always quote items in lil_list_to_value Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 26/28] cli: lil: Allocate len even when str is NULL in alloc_value_len Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 27/28] cli: lil: Add a function to quote values Sean Anderson
2021-07-01  6:16 ` [RFC PATCH 28/28] cli: lil: Load procs from the environment Sean Anderson
2021-07-01 20:21 ` [RFC PATCH 00/28] cli: Add a new shell Tom Rini
2021-07-02 11:30   ` Wolfgang Denk
2021-07-02 13:56     ` Sean Anderson
2021-07-02 14:07   ` Sean Anderson
2021-07-08  3:49 ` Heiko Schocher
2021-07-08  4:26   ` Sean Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=163090.1625507434@gemini.denx.de \
    --to=wd@denx.de \
    --cc=badsector@runtimeterror.com \
    --cc=marek.behun@nic.cz \
    --cc=rasmus.villemoes@prevas.dk \
    --cc=roland.gaudig-oss@weidmueller.com \
    --cc=seanga2@gmail.com \
    --cc=sjg@chromium.org \
    --cc=steveb@workware.net.au \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    --cc=xypron.glpk@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox