From owner-svn-src-head@freebsd.org Mon Nov 6 05:33:28 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 1AD70E4B413; Mon, 6 Nov 2017 05:33:28 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id B64F67A1; Mon, 6 Nov 2017 05:33:27 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.104] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id 30C44D627BF; Mon, 6 Nov 2017 16:03:35 +1100 (AEDT) Date: Mon, 6 Nov 2017 16:03:34 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Ed Maste cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r325444 - head/sys/kern In-Reply-To: <201711051949.vA5JniU4037526@repo.freebsd.org> Message-ID: <20171106144303.P987@besplex.bde.org> References: <201711051949.vA5JniU4037526@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=cK6QihWN c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=wCP3Pv7mfX1hg_B8BXEA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Nov 2017 05:33:28 -0000 On Sun, 5 Nov 2017, Ed Maste wrote: > Log: > ANSIfy sys/kern/md4c.c > > PR: 223453 > Submitted by: ota@j.email.ne.jp > MFC After: 2 weeks This doesn't ANSIfy md4c.c, but churns its style away from both the vendor version and the copy in libmd. The style is very non-KNF, but is preserved to avoid churning. md4c.c already had prototypes, by declaring extern ones in the headers (there are further bugs from inconsistent churning in the headers. includes , but renames its functions using excessively ifdefed defines, so its functions don't actually have the name in their prototypes; repeats everything in except and then makes them inconsistent but still uses macros to obfuscate the names. The inconsistencies are: - libmd MD4Update and MD4Data take a const void *, but kernel MD4Update and MD4Data take a const unsigned char * - libmd MD4Final takes an unsigned char [16], but kernel MD4Final takes an unsigned char [__min_size(16)]; old versions hard-coded __min_size() and failed to compile with C90 and earlier compilers (also with the distribution version of gcc-4.2.1 -std=c99, since it implements n869.txt and not C99 in this area); now the kernel version is only the same as the libmd version for compilers that don't implement C99. C99 - user parts of sys/md4.h are under !_KERNEL . This is bogus since only md4.h should be used if !_KERNEL. These parts include MD4Final which has inconsistent prototypes, so including both headers would fail. - sys/md4 doesn't declare the FreeBSD extensions MD4Fd, MD4FdChunk and MD4FileChunk even under its bogus ifdef. > Modified: head/sys/kern/md4c.c > ============================================================================== > --- head/sys/kern/md4c.c Sun Nov 5 19:38:51 2017 (r325443) > +++ head/sys/kern/md4c.c Sun Nov 5 19:49:44 2017 (r325444) Before this commit, diffs with libmd/md4c.c were only 97 lines, with less than half the changes for churning and other style bugs. Some are: - __FBSDID() is misplaced in the libmd version - only the libmd version is obfuscated using the CONST_POINTER typedef. Both are obfuscated using the POINTER typedef. (POINTER was to make RSA md portable to pre-C90 compilers. It is harmlessly wrong on C90 for const pointers, except -Wcast-qual detects the error and -Werror makes it harmfully wrong. The correct cast for C90 with prototypes is none, to let the protoype convert.) - only the libmd version has the good name 'input' for a parameter weakened to 'in'. This might be related to the next renaming. - only the libmd version has the not so good name 'index' for a parameter changed to 'idx'. The kernel no longer has index(), so the name 'index' no longer cause -Wshadow warnings. -Wshadow has never been used for the the kernel, so these warnings didn't occur even when the kernel had index(). index() tends to get declared everywhere in libmd and the kerenel via namespace pollution in and , respectively. Most of the diffs are from renaming 'index'. - only the libmd version churns bcopy() to memcpy(). This also adds bogus casts to POINTER and CONST_POINTER. These casts are more bogus for memcpy where they are used than for bcopy where they are not used, since pre-C90 had bcopy() where the casts were needed but might not have memcpy() where the casts are not needed for C90 and later. - similarly to churn bzero() to memset(), except bzero() is better instead of different, and the churning also changes the parentheses style from KNF to gnu. - only the kernel version churns and breaks MD4Final(). It "ANSIfies" the function, apparently to change [16] to [static 16]. This is not ANSI. ANSI C is the earlier American version of C90. [static 16] is a syntax error in C90. The kernel md4c.c is only used by netsmb. I don't use netsmb, else this syntax error would break compiling with gcc-4.2.1. Altogther, only about 20 lines of the 97 lines diffs are needed (mainly 14 lines to add weak references). > @@ -90,8 +90,8 @@ static unsigned char PADDING[64] = { > > /* MD4 initialization. Begins an MD4 operation, writing a new context. > */ > -void MD4Init (context) > -MD4_CTX *context; /* context */ > +void > +MD4Init(MD4_CTX *context) "ANSIficication" also loses comments on parameter names. This is an improvements for banal comments like this, except it is a style bug. RSA style apparently requires comments. This also changes from gnu style to KNF for parentheses. Apparently the parentheses style change that accompanies churning bzero to memset is a style fix for some previous change involving bzero. > @@ -107,10 +107,9 @@ MD4_CTX *context; > operation, processing another message block, and updating the > context. > */ > -void MD4Update (context, input, inputLen) > -MD4_CTX *context; /* context */ > -const unsigned char *input; /* input block */ > -unsigned int inputLen; /* length of input block */ The comments are more useful here. In libmd, they are even more useful since the connection between input and inputlen is broken by renaming only the former. > @@ -164,7 +163,8 @@ MD4_CTX *context; > /* MD4 finalization. Ends an MD4 message-digest operation, writing the > the message digest and zeroizing the context. > */ > -void MD4Final (unsigned char digest[static 16], MD4_CTX *context) > +void > +MD4Final(unsigned char digest[static 16], MD4_CTX *context) This was already "ANSIfied". The churn is from gnu/RSA parentheses to KNF (as above), and also from RSA style for function types to KNF. Even gnu style puts the function name on a new line. Similarly for other changes. > @@ -254,10 +253,8 @@ const unsigned char block[64]; > /* Encodes input (UINT4) into output (unsigned char). Assumes len is > a multiple of 4. > */ > -static void Encode (output, input, len) > -unsigned char *output; > -UINT4 *input; > -unsigned int len; > +static void > +Encode(unsigned char *output, UINT4 *input, unsigned int len) Another bug in the libmd version is that it only renames 'input' to 'in' in one function. Many functions still have a parameter named 'input', so the change can't be for fixing a -Wshadow problem. Bruce