svn commit: r325444 - head/sys/kern

Bruce Evans brde at optusnet.com.au
Mon Nov 6 05:33:28 UTC 2017


On Sun, 5 Nov 2017, Ed Maste wrote:

> Log:
>  ANSIfy sys/kern/md4c.c
>
>  PR:		223453
>  Submitted by:	ota at 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


More information about the svn-src-all mailing list