Date: Mon, 6 Nov 2017 16:03:34 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Maste <emaste@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r325444 - head/sys/kern Message-ID: <20171106144303.P987@besplex.bde.org> In-Reply-To: <201711051949.vA5JniU4037526@repo.freebsd.org> References: <201711051949.vA5JniU4037526@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
<md5.h> includes <sys/md5.h>, but renames its functions using
excessively ifdefed defines, so its functions don't actually have the
name in their prototypes; <sys/md4.h> repeats everything in <md4.h>
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 <string.h> and <sys/systm.h>,
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20171106144303.P987>