From owner-svn-src-all@FreeBSD.ORG Thu Apr 3 07:52:04 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 44B1AB82; Thu, 3 Apr 2014 07:52:04 +0000 (UTC) Received: from mail104.syd.optusnet.com.au (mail104.syd.optusnet.com.au [211.29.132.246]) by mx1.freebsd.org (Postfix) with ESMTP id 020FB808; Thu, 3 Apr 2014 07:52:02 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail104.syd.optusnet.com.au (Postfix) with ESMTPS id 1219D422B31; Thu, 3 Apr 2014 18:51:51 +1100 (EST) Date: Thu, 3 Apr 2014 18:51:43 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: d@delphij.net Subject: Re: svn commit: r264059 - head/bin/dd In-Reply-To: <533D0307.6090509@delphij.net> Message-ID: <20140403183237.I1546@besplex.bde.org> References: <201404030055.s330tGCI070764@svn.freebsd.org> <20140403150420.H959@besplex.bde.org> <533D0307.6090509@delphij.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=U6SrU4bu c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=pzt8HfeZUOQA:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=9eILa-v36Y3XtNAoasQA:9 a=CjuIK1q_8ugA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Xin LI X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Apr 2014 07:52:04 -0000 On Wed, 2 Apr 2014, Xin Li wrote: > On 4/2/14, 9:45 PM, Bruce Evans wrote: >> On Thu, 3 Apr 2014, Xin LI wrote: >>> 0x00020 -#define C_FILES 0x00040 -#define C_IBS >>> 0x00080 -#define C_IF 0x00100 -#define C_LCASE >>> 0x00200 -#define C_NOERROR 0x00400 -#define C_NOTRUNC >>> 0x00800 -#define C_OBS 0x01000 -#define C_OF >>> 0x02000 -#define C_OSYNC 0x04000 -#define C_PAREVEN >>> 0x08000 -#define C_PARNONE 0x100000 >> >> The previous formatting was not too good, and obfuscated a bug >> here. 4 bits are unused. Someone added this bug together with many >> others in 2004, and I missed it then when I committed a fix for the >> others. Most of the bugs that I fixed were unsorting of the table >> by adding to its end. Insertion sort tends to give large churn of >> the table by changing all the numbers, but for the 2004 fix there >> wasn't much churn except relative to the unsorted version because >> all the changes were near the end. Actually it was me who added the unused bits. *Blush*. I see you committed a fix. >>> -#define C_PARODD 0x200000 -#define C_PARSET >>> 0x400000 -#define C_SEEK 0x800000 -#define C_SKIP >>> 0x1000000 -#define C_SPARSE 0x2000000 -#define C_SWAB >>> 0x4000000 -#define C_SYNC 0x8000000 -#define C_UCASE Mail programs keep getting worse. Here they destroyed the line wrapping together with rearranging the quotes. > Will it be an improvement if we have the attached change committed? > Basically it redo the sort and use 1 <<'s instead of hardcoded heximal > numbers. Too much style changing. dd still uses hex constants for smaller flag values elsewhere, even in the same file. >> The 2 values above UINT_MAX also won't compile with compilers that >> warn that 'integer constant is too large for "long" type'. Mainly >> gcc > without >> -std=c99. clang is too incompatible to warn about this even with >> -std=c89 -Wall. The fix is not to further churn the table by >> adding a ULL suffix to all entries. Using shifts gives a similar type problem for 32-bit types. From the patch: % -#define C_NOXFER 0x10000000 % -#define C_NOINFO 0x20000000 % ... % +#define C_UCASE (1 << 28) % +#define C_UNBLOCK (1 << 29) This works up to (1 << 30), assuming 32-bit ints, but (1 << 31) would overflow. It would have to be written as (1U << 31) (keep assuming 32-bit ints), but this would mess up the fancy formatting. Elswhere in the patch: % -#define C_ASCII 0x00000001 % -#define C_BLOCK 0x00000002 % ... % +#define C_ASCII (1 << 0) % +#define C_BLOCK (1 << 1) Fancy formatting like this (the extra space before the shift count to line things up) is hard to maintain since it is too fancy for indent(1) to produce or preserve. Bruce