From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EB26DC07E9B for ; Mon, 5 Jul 2021 15:32:43 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7E79E61998 for ; Mon, 5 Jul 2021 15:32:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7E79E61998 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 6774682C54; Mon, 5 Jul 2021 17:31:02 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="nVS4r7dy"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 4678C82C48; Mon, 5 Jul 2021 17:30:59 +0200 (CEST) Received: from mail-oi1-x22d.google.com (mail-oi1-x22d.google.com [IPv6:2607:f8b0:4864:20::22d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 07B8682C47 for ; Mon, 5 Jul 2021 17:30:55 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-oi1-x22d.google.com with SMTP id q23so21064989oiw.11 for ; Mon, 05 Jul 2021 08:30:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=qESM7/zBniszUIbhrPWhvvwf0JPCYhyGyEhT1OkKq8c=; b=nVS4r7dyGQtVUb4iE4G/trh+R1GLhqrsQOpf21gS2hZhY0uw+dY6KkGBY/w9TNrBeW 5IEaQ1bEWzHKJ8wUE/kY3cOrp4gh/h3blCNzT2BJva5ikrfyUkfC+lOF+FiZ36fgqvV5 9+5ZaGArCBJzvId2rO1wMgM/L3YrbFdglmepQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qESM7/zBniszUIbhrPWhvvwf0JPCYhyGyEhT1OkKq8c=; b=JqHANG5XrSclHHdXKYlOgo96neFaeXZEh+o2BFN5hHEs2xUsGQfro11Qb+IYI2HtM8 mNqS1xx3jfvL/2BhcmWp8v7TnV09JQXCDtx1XgEOyvX+RDYNX98G7JOqYZYX4yeZpplS e7f9wRFcy467VsciInsmBExOPc0oZ4qeJOhrHL7WvjkMHpfX6Qr6kI98XeMMEuCkPiUE ueqgPAyFuPyqbsqvYjq1pLdZPQA1RT8RGLYbNps1Ybw4re/9TgolluogZ1OWMU/vrI/E Q7z1fcV+qdhx82WGEHDNlg64FDZI0p0JvGAgtM+Fc/TsfQ++YJW/Yz8mjhOYsw58mdRV dQ5w== X-Gm-Message-State: AOAM530fDDUO3b4QGRu6ARWvCSN1OueBlFtIeI+9DphUn/s9YKdhuuNX vSHt23LYBD7Oo+pQiYp7FiruKF2HxVCBWThvOM7nIA== X-Google-Smtp-Source: ABdhPJwmaCjcG3P+2lwttXY7v7wXACsxS9JZgUoh5ksO/VjlduCS/aaAwfrSZHHK+RmlSpoUwmj1FuQ4iOB7/jAeqUA= X-Received: by 2002:a05:6808:910:: with SMTP id w16mr10375432oih.53.1625499053313; Mon, 05 Jul 2021 08:30:53 -0700 (PDT) MIME-Version: 1.0 References: <20210701061611.957918-1-seanga2@gmail.com> <20210701061611.957918-4-seanga2@gmail.com> <17176.1625340369@gemini.denx.de> <54A6EFA6-8D5B-4779-B344-87BCC8C14B9C@workware.net.au> <5a967151-94f0-6037-2d02-0114c43b846c@gmail.com> In-Reply-To: <5a967151-94f0-6037-2d02-0114c43b846c@gmail.com> From: Simon Glass Date: Mon, 5 Jul 2021 09:29:46 -0600 Message-ID: Subject: Re: [RFC PATCH 03/28] cli: lil: Replace strclone with strdup To: Sean Anderson Cc: Steve Bennett , Wolfgang Denk , Rasmus Villemoes , U-Boot Mailing List , Tom Rini , =?UTF-8?B?TWFyZWsgQmVow7pu?= , Roland Gaudig , Heinrich Schuchardt , Kostas Michalopoulos Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi, On Mon, 5 Jul 2021 at 08:42, Sean Anderson wrote: > > On 7/5/21 1:07 AM, Steve Bennett wrote: > > On 4 Jul 2021, at 5:26 am, Wolfgang Denk wrote: > >> > >> Dear Sean, > >> > >> In message you wrote: > >>> > >>> Well, since Hush was never updated, I don't believe LIL will be either. > >> > >> Let's please be exact here: Hus has never been updated _in_U-Boot_, > >> but it has seen a lot of changes upstream, which apparently fix all > >> the issues that motivated you to look for a replacement. > >> > >>> I think reducing the amount of ifdefs makes the code substantially > >>> easier to maintain. My intention is to just use LIL as a starting point > >>> which can be modified as needed to better suit U-Boot. > >>> > >>> The other half of this is that LIL is not particularly actively > >>> developed. I believe the author sees his work as essentially > >>> feature-complete, so I expect no major features which we might like to > >>> backport. > >> > >> This sounds like an advantage, indeed, but then you can also > >> interpret this as betting on a dead horse... > > > > My 2c on this. > > > > I am the maintainer of JimTcl (and I agree it is too big to be considered a candidate). > > LIL source code has almost zero comments, poor error checking and no test suite. > > FWIW I added a (small) test suite in "[RFC PATCH 17/28] test: Add tests > for LIL" based on the tests included in the LIL distibution. However, I > really would like to expand upon it. > > > I would be very hesitant to adopt it in u-boot without serious work. > > I think around half of the "serious work" has already been done. I have > worked on most of the core of LIL, and added error handling and > comments. I believe that most of the remaining instances of dropping > errors lie in the built-in commands. > > > I would much rather see effort put into updating hush to upstream. > > 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. > > > My guess is that Denys would be amenable to small changes to make it easier to synchronise > > with busybox in the future. > > 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 > > * 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. > > * 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. > > * Tools such as grep, cut, tr, sed, sort, uniq, etc. which are extremely > useful when working with streams are not present in U-Boot. > > And of course, this feature is currently not present in U-Boot. To get > around this, commands resort to two of my least-favorite hacks: passing > in the name of a environmental variable and overloading the return > value. For an example of the first, consider > > => part uuid mmc 0:1 my_uuid > > 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. > > The other way to return more information is to use the return value. > Consider the button command; it currently returns > > 0 ON, the button is pressed > 1 OFF, the button is released > 0 button list was shown > 1 button not found > 1 invalid arguments > > and so there is no way to distinguish between whether the button is off, > whether the button does not exist, or whether there was a problem with > the button driver. > > 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 > 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. I think these are good points, particularly the thing about mutiltasking. In fact I think it would be better if hush could implement things like '| grep xxx' since it would be useful in U-Boot. But in any case, adding lil does not preclude someone coming along and adaptive hush for U-Boot (and this time upstreaming the changes!). I have seen discussions about updating hush for several years and no one has done it. I took a quick look and found it was about 3x the size it used to be and was not even sure where to start in terms of adapting it for single-threaded use. Re this patch, I think it should be sent upstream. Regards, Simon