Re: [netmod] Benjamin Kaduk's Discuss on draft-ietf-netmod-artwork-folding-11: (with DISCUSS and COMMENT)

Erik Auerswald <auerswal@unix-ag.uni-kl.de> Sat, 11 January 2020 14:25 UTC

Return-Path: <auerswal@unix-ag.uni-kl.de>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 599E212008F; Sat, 11 Jan 2020 06:25:17 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.897
X-Spam-Level:
X-Spam-Status: No, score=-1.897 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, MAY_BE_FORGED=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_NONE=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id r9hi7S4vR9Ca; Sat, 11 Jan 2020 06:25:14 -0800 (PST)
Received: from mailgw1.uni-kl.de (mailgw1.uni-kl.de [IPv6:2001:638:208:120::220]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id B58E3120045; Sat, 11 Jan 2020 06:25:14 -0800 (PST)
Received: from [172.20.10.2] (x2e720344.dyn.telefonica.de [46.114.3.68] (may be forged)) (authenticated bits=0) by mailgw1.uni-kl.de (8.14.4/8.14.4/Debian-8+deb8u2) with ESMTP id 00BEOxai109513 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Sat, 11 Jan 2020 15:25:09 +0100
To: Benjamin Kaduk <kaduk@mit.edu>, The IESG <iesg@ietf.org>
Cc: draft-ietf-netmod-artwork-folding@ietf.org, Lou Berger <lberger@labn.net>, netmod-chairs@ietf.org, netmod@ietf.org
References: <157861251031.11821.9719140738474484600.idtracker@ietfa.amsl.com>
From: Erik Auerswald <auerswal@unix-ag.uni-kl.de>
Message-ID: <597a69b9-a7ec-6b63-65e1-0c64d63574f5@unix-ag.uni-kl.de>
Date: Sat, 11 Jan 2020 15:24:57 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2
MIME-Version: 1.0
In-Reply-To: <157861251031.11821.9719140738474484600.idtracker@ietfa.amsl.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-GB
Content-Transfer-Encoding: 7bit
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/00OspYYHdZwwdkI-ihl6ohfcUHQ>
Subject: Re: [netmod] Benjamin Kaduk's Discuss on draft-ietf-netmod-artwork-folding-11: (with DISCUSS and COMMENT)
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 11 Jan 2020 14:25:17 -0000

Hello Benjamin,

On 10.01.20 00:28, Benjamin Kaduk via Datatracker wrote:
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-netmod-artwork-folding-11: Discuss
> 
> When responding, please keep the subject line intact and reply to all
> email addresses included in the To and CC lines. (Feel free to cut this
> introductory paragraph, however.)
> 
> 
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-netmod-artwork-folding/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Thank you for the updates in the -10 and -11; the content looks a lot better and
> I am not uncomfortable about publishing as Informational (vs. BCP)!

Thanks for the review. :-)

> That said, I think the edits to the script have introduced a regression:
> 
>       # ensure input file doesn't contain the fold-sequence already
>       if [[ -n "$("$SED" -n '/\\$/{N;s/\\\n[ ]*\\/&/p}' "$infile")" ]]
> 
> Unfortunately, I'm not sure this gets all cases, since the 'N' command
> reads a line and prevents it from being considered as the first half of the
> wrapped sequence:
> 
> kaduk$:~/git/openssl$ cat /tmp/a
> this is a line\
> another line\
> \that wraps
> kaduk$:~/git/openssl$ cat /tmp/b
> this is a line
> another line\
> \that wraps
> kaduk$:~/git/openssl$ sed -n '/\\$/{N;s/\\\n[ ]*\\/&/p}' < /tmp/a
> kaduk$:~/git/openssl$ sed -n '/\\$/{N;s/\\\n[ ]*\\/&/p}' < /tmp/b
> another line\
> \that wraps

Thanks for the bug report complete with test cases and analysis. :-)
The fix should be adding ";D" to the sed script:

     sed -n '/\\$/{N;s/\\\n[ ]*\\/&/p;D}'

I'll look into adding this (with tests) soon.

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> A few other comments from reviewing the version of the script in the -11:
> 
> When processing input, it's perhaps more robust to check $# before assigning $2
> to a named parameter.

That sounds reasonable.

>       printf "Exit status code: 1 on error, 0 on success, 255 on no-op."
> 
> Interesting to have no newline here but two on the next line's printf, but I
> guess it might be at the column limit already.

Exactly.

> (The quotes on 'Error'/'Warning'/'Debug' in err()/warn()/dbg() are noops.)

They improve syntax-highlighting results in vim. ;-)

>     # warn if a non-GNU sed utility is used
>     "$SED" --version < /dev/null 2> /dev/null \
>     | grep GNU > /dev/null 2>&1 || \
> 
> `grep -q` should be usable instead of `grep >/dev/null`

I intend to change this to use `grep -q`.

Thanks,
Erik