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=-4.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 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 86FB3C07E9B for ; Mon, 5 Jul 2021 15:43:02 +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 0857B61968 for ; Mon, 5 Jul 2021 15:43:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0857B61968 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com 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 36F9D800AA; Mon, 5 Jul 2021 17:43:00 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="ejOH2LUG"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 56EDB82BCE; Mon, 5 Jul 2021 17:42:58 +0200 (CEST) Received: from mail-qt1-x831.google.com (mail-qt1-x831.google.com [IPv6:2607:f8b0:4864:20::831]) (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 BE3EE8009C for ; Mon, 5 Jul 2021 17:42:54 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=seanga2@gmail.com Received: by mail-qt1-x831.google.com with SMTP id d1so2784482qto.4 for ; Mon, 05 Jul 2021 08:42:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=VzEJIdFEOXPoeKhFyI0nTfIxIbOr6n/iseW95pvpfk8=; b=ejOH2LUG5KS0/e7e6mJPmWxfufhBpX+xntixLE5CmRMQgvf6fj1OTj2imHO1HZdFWF FAad+cpUfSFZztu3oWIF5TrFfei7BDgkwyjMZhWKeMXanNAKmMpcDlA00qjfMyZ+Z0iH scEzsDWw8znuO0CoXEiQktPRzAt615SrYBHT+c/YAZQtZbybG6GNBEkyrtPmTBsypozs D6YAXqHyFIHHbQqUoa4lZ/o3OiybJpsWDVyaxfy7uOADLqYvRazBquvxLViEdcvk20Rk Hq1Ttkb+IkuA4k7NHPdn+5rQY9ODawZOumLPskKltAveDxcIc4n2PmguhHkCfRn7wSVi kLXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=VzEJIdFEOXPoeKhFyI0nTfIxIbOr6n/iseW95pvpfk8=; b=askUUNTBeyctg/vOjOUNZZum/WafN27Ltm+8dZyTXSXW782sMd7FiXoeEENc4HjNiV 0X5/AlaboFXVzVGilRsAoyMOBsZ9Ubb6U86VDz2nBYRlr+ljRybu9LnhKYZbAvOoYkW0 4P9QWOkkU5adcNMX3Jp692aXzKxlBHvq4NAGRE9d1/3nQBk6OXiot592CEKx/DtnO29K YGI6S3nhdQGRsB+w6JkBCq/UbTYQofCeFxjV4VBdq4Wl4gGQ/Qs2cdC/3XwTdAPwL1yN hSGD3syFB/bcvSRPK+TJy4Mr1RnoxeqBmp4tPaJ1sFkzp9Y588pevCqVTkpSYZcuR4hZ 6Gug== X-Gm-Message-State: AOAM533vKnagLbOEAgDAinXe6Wfye3/vthH5qlDUnMybc5FOeGvQZVgi hlLeuUINasOa96lDeo1caB0= X-Google-Smtp-Source: ABdhPJxjyzQgczC8xx3v78g31q3nw9Bn7EnEj7v875FFZLIUaYdC7F9IT90d9EkCJzA8ezuKBr6bzw== X-Received: by 2002:ac8:5803:: with SMTP id g3mr7515196qtg.3.1625499773536; Mon, 05 Jul 2021 08:42:53 -0700 (PDT) Received: from [192.168.1.201] (pool-74-96-87-9.washdc.fios.verizon.net. [74.96.87.9]) by smtp.googlemail.com with ESMTPSA id x14sm3328666qta.90.2021.07.05.08.42.52 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 05 Jul 2021 08:42:53 -0700 (PDT) Subject: Re: [RFC PATCH 03/28] cli: lil: Replace strclone with strdup To: Simon Glass Cc: Steve Bennett , Wolfgang Denk , Rasmus Villemoes , U-Boot Mailing List , Tom Rini , =?UTF-8?Q?Marek_Beh=c3=ban?= , Roland Gaudig , Heinrich Schuchardt , Kostas Michalopoulos 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> From: Sean Anderson Message-ID: Date: Mon, 5 Jul 2021 11:42:52 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 On 7/5/21 11:29 AM, Simon Glass wrote: > 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. I believe it will not be accepted, since strdup is POSIX and not C99. --Sean