From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10956 invoked by alias); 4 Sep 2016 09:38:25 -0000 Mailing-List: contact cygwin-help@cygwin.com; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cygwin-owner@cygwin.com Mail-Followup-To: cygwin@cygwin.com Received: (qmail 10936 invoked by uid 89); 4 Sep 2016 09:38:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=2.1 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=kitchen, portability, eating, U*cygsimple X-HELO: mail-it0-f42.google.com Received: from mail-it0-f42.google.com (HELO mail-it0-f42.google.com) (209.85.214.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 04 Sep 2016 09:38:14 +0000 Received: by mail-it0-f42.google.com with SMTP id c198so107008124ith.1 for ; Sun, 04 Sep 2016 02:38:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=Lhcngh9MXHlN96UWpRSQg2BnPNSjcXA/gkFSQch0DGE=; b=XSLI2v9YYNd/ujAq87yObgCIofyajr2t8J/tkt6Pa7hJtwv4QCCN0CWsYV+oNdvFj7 qsn6XxMHH4e9rPSFBeKXhdRtJ4Vo0LwObhDgw7DL7mwaBxb2YLChFywaBHx4k2b+Z1uN ILaNmQPbFUEId6+kgfodoMyGaTq7DkUHuIxYwsSfHSqkRryRf+GyGjpiGwa04oAeOU64 ZllcwZj8GI0IDtbW6skrb98sUzDJ3roVEOpI5muU+yL6/geO5ESK3R/nOFtMthArTHg0 XbVu4HB2tF9nb7jTF6FU87qSjD7eJOqlVnoAG1o3ZYh2/RhoHHp+cVH7+mgmreobhxfj 9Dag== X-Gm-Message-State: AE9vXwNmjdmK9rdx+BuvYtzegjaQZ82s+xuxp6F9cwGhyxtsFyHfXRw51WB5ObWOcVcdDDbRuS5ErEnSccs3Rg== X-Received: by 10.36.25.144 with SMTP id b138mr14641649itb.29.1472981892347; Sun, 04 Sep 2016 02:38:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.116.26 with HTTP; Sun, 4 Sep 2016 02:38:11 -0700 (PDT) In-Reply-To: <7314ffba-a927-f565-e38e-7454d6c2ef0f@gmail.com> References: <93be816b-952c-20cd-575e-940cdf4fbbd1@redhat.com> <1de2efdc-b26c-4914-580c-1a640d0a46fd@redhat.com> <7314ffba-a927-f565-e38e-7454d6c2ef0f@gmail.com> From: Gene Pavlovsky Date: Sun, 04 Sep 2016 09:38:00 -0000 Message-ID: Subject: Re: Script broken after updating bash to 4.3.46-7? To: cygwin@cygwin.com Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2016-09/txt/msg00058.txt.bz2 This issue never affected me personally, but it sounds like a serious bug and I'm as glad as anybody that it's finally fixed. However, having existing scripts suddenly breaking is not great. I'd like to remind that I'm not the author of the script in question, [AutoMySQLBackup](http://sourceforge.net/projects/automysqlbackup/). If I put "| d2u" there I'll have to remember it and do it every time automysqlbackup is updated - or create and maintain a Cygwin package for this script. And who knows how many other scripts might be broken, I just didn't find it yet? Having a `read`-specific shell option telling read to treat `\r\n` (and only `\r\n`, not \r followed by something else) same as `\n` would be bad things to have? For me, this kind of option would be more useful than the `igncr` crutch Let me say it another way - in OOP programming, one of good practices is Single Responsibility Principle - a class should be responsible for only one feature/function, and that feature/function should be totally encapsulated in that class. Similar to that, an option should be responsible for one behavior. With this change to `read`, the `igncr` shell option is starting to look like a kitchen sink... split it into separate options, please! I think making UNIX scripts work on Cygwin with no or minimum modifications (or bug-hunting) should be one of high priorities, no? If some scripts erroneously have CRLF line endings, it's easy to find and `d2u` them, rather than using the `igncr` crutch, but with the recent change to `read`, countless scripts might be broken in a non-obvious way. Fixing them would require finding out they're broken, in the first place. Imagine if I didn't set up my cron to e-mail me the cron jobs output? My backup script would just stop working silently, and some time later when I needed a recent backup, I would find out there aren't any. There might be something else lurking that I haven't found yet. Once a script, broken by this change to `read`, is found, it must be checked thoroughly to find out where exactly is the problem, where to put '| d2u', or maybe 'set -o igncr'. These fixes must also be applied anytime a 3rd party script is updated. Quite a lot of work! Hope you will consider my point. Regards, Gene. On 30 August 2016 at 23:57, cyg Simple wrote: > On 8/30/2016 1:38 PM, Eric Blake wrote: >> On 08/30/2016 12:04 PM, cyg Simple wrote: >>> On 8/29/2016 2:30 PM, Eric Blake wrote: >>>> >>>> Simplest fix: >>>> >>>> read ... < <(mysql ... | dos2unix) >>>> >>> >>> This will break when the data returned by mysql is supposed to contain \r. >>> >>>> There. Now you aren't feeding \r to read in the first place. >>>> >>> >>> But you might want to feed \r to read. It isn't a fix, it is a >>> potential work around dependent on the data set results. If a read that >>> is supposed to be reading binary data doesn't pass all of the data to >>> the routine then it is broken. >> >> Now we're talking past each other. >> >> That's what the recent bash fixed. 'read' in bash 3.2.42-4 was broken - >> it corrupted binary data, with no recourse, by eating \r (and worse, by >> sometimes eating the byte after \r). 'read' in bash 3.2.46-7 is fixed - >> by default it is strictly binary (all bytes are read as-is, including >> \r), but can also be switched to text mode (using 'igncr', all \r are >> ignored). If you want to preserve mid-line \r but treat line endings of >> \r\n as a single byte, then leave binary mode on and strip the line >> endings via a separate tool like d2u (note, however, that it is very >> rare to have data where mid-line \r is important but line-ending \r\n >> should be treated as plain \n). >> >> I strongly think that using igncr is a crutch, and you normally >> shouldn't use it; particularly not if you want to be portable to other >> platforms. Instead, massaging your data through d2u is a great way to >> be portable. But sometimes the ease of ignoring ALL \r is easier than >> worrying about portability, so I keep the 'igncr' code in Cygwin. >> >> And it is only because the OP tried using 'igncr' in the first place >> (whether or not it was actually needed) that we have now flushed out the >> existence of a latent bug in the 'igncr' implementation that interacts >> weirdly with $()\n in PS1. On that front, I'm still hoping to find time >> to debug and/or for someone to post a patch. But whether PS1 behaves >> weirdly under 'igncr' is orthogonal to my suggestion above - using >> 'mysql|d2u' is a great way to avoid the need to worry about 'igncr'. >> > > Thank you for the retort Eric. Happy to know that it is fixed which in > the back of my mind I knew already. I can imagine data such as full > message email or a small document data containing \r\n as valid data in > the database field and if you use a line ending conversion utility you > might loose that data. > > -- > cyg Simple > > -- > Problem reports: http://cygwin.com/problems.html > FAQ: http://cygwin.com/faq/ > Documentation: http://cygwin.com/docs.html > Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple > -- Problem reports: http://cygwin.com/problems.html FAQ: http://cygwin.com/faq/ Documentation: http://cygwin.com/docs.html Unsubscribe info: http://cygwin.com/ml/#unsubscribe-simple