List:Internals« Previous MessageNext Message »
From:Michael Widenius Date:May 19 2002 11:39am
Subject:bk commit into 4.1 tree
View as plain text  
Hi!

>>>>> "peter" == peter  <peter@stripped> writes:

peter> ChangeSet
peter>   1.1243 02/05/10 12:12:10 peter@stripped +10 -0
peter>   Add  SHA1 and SHA functions which both corresponds to SHA1 digest message 
peter>   generation.

<cut>

peter> --- 1.45/sql/item_strfunc.cc	Thu Apr 18 10:53:55 2002
peter> +++ 1.46/sql/item_strfunc.cc	Fri May 10 12:12:00 2002

<cut>

peter> +
peter> +
peter> +
peter> +/* Functions for SHA1 Function object. */
peter> +
peter> +
peter> +

When it comes to function comments, I have tried to have 2 empty lines
before a function comment and one empty line afterwords.

peter> +String *Item_func_sha::val_str(String *str)
peter> +{
peter> +  String * sptr= args[0]->val_str(str);
peter> +  if (sptr)  /* If we got value different from NULL */
peter> +  {
peter> +    SHA1Context context;  /* Context used to generate SHA1 hash */
peter> +    uint8_t digest[SHA1HashSize];   /* Temporary buffer to store 160bit digest
> */
peter> +    int err;              /* error code for SHA library functions */
peter> +    
peter> +    null_value=0;
peter> +    SHA1Reset(&context);  /* We do not have to check for error here */
peter> +    SHA1Input(&context,(const unsigned char *) sptr->ptr(),
> sptr->length());
peter> +    /* No need to check error as the only case would be too long message */

For which function call can we in theory get an error. SHA1Input() or
SHA1Result() ?

peter> +    err=SHA1Result(&context,digest);

<cut>

peter>  /*
peter>  ** Concatinate args with the following premissess

The multi-line comment syntax we should use are as follows:

/*
  Comment
*/

Not

/*
** Comment
*/

(Yes, I know we have to correct a lot of old code regarding this...)

peter> +++ include/sha1.h	02/05/10 12:12:00

<cut>

peter> #ifndef _SHA1_H_
peter> #define _SHA1_H_

peter> #include <stdint.h>
peter> /*
peter>  * If you do not have the ISO standard stdint.h header file, then you
peter>  * must typdef the following:
peter>  *    name              meaning
peter>  *  uint32_t         unsigned 32 bit integer
peter>  *  uint8_t          unsigned 8 bit integer (i.e., unsigned char)
peter>  *  int_least16_t    integer of >= 16 bits
peter>  *
peter>  */


Instead of the above typedefs, you should use the following one that
are always defined by my_global.h:

uint32
int16
uint8

This will remove the dependenc of stdint.h, which may not exists on
all systems.


peter> #ifndef _SHA_enum_
peter> #define _SHA_enum_

Why do you need the above define ?
If you are concerned it's defined by some standard include file, you
should rename the enum to something else.

peter> enum
peter> {
peter>     shaSuccess = 0,
peter>     shaNull,            /* Null pointer parameter */
peter>     shaInputTooLong,    /* input data too long */
peter>     shaStateError       /* called Input after Result */
peter> };
peter> #endif
peter> #define SHA1HashSize 20

Instead of doing an unamed enum, you should use a named one:

enum sha_result_codes
{
    shaSuccess = 0,
    shaNull,            /* Null pointer parameter */
    shaInputTooLong,    /* input data too long */
    shaStateError       /* called Input after Result */
};

peter> --- New file ---
peter> +++ mysys/sha1.c	02/05/10 12:12:01

peter>  *      SHA-1 is designed to work with messages less than 2^64 bits
peter>  *      long.  Although SHA-1 allows a message digest to be generated
peter>  *      for messages of any number of bits less than 2^64, this
peter>  *      implementation only works with messages with a length that is
peter>  *      a multiple of the size of an 8-bit character.

What does the above mean in practice?
That this version only works on 8 bit machines ?

peter> int SHA1Result( SHA1Context *context,
peter>                 uint8_t Message_Digest[SHA1HashSize])
peter> {
peter>     int i;

peter>     if (!context || !Message_Digest)
peter>     {
peter>         return shaNull;
peter>     }

I would like to have the above surrounded by an #ifndef DBUG_OFF test.
(mysys should be able to normally assume that the caller knows what to do)

<cut>

peter> int SHA1Input(    SHA1Context    *context,
peter>                   const uint8_t  *message_array,
peter>                   unsigned       length)
peter> {
peter>     if (!length)
peter>     {
peter>         return shaSuccess;
peter>     }

peter>     if (!context || !message_array)
peter>     {
peter>         return shaNull;
peter>     }

The above test is not needed (or should be ifdef-ed)


peter>     if (context->Computed)
peter>     {
context-> Corrupted = shaStateError;

peter>         return shaStateError;
peter>     }

peter>     if (context->Corrupted)
peter>     {
peter>          return context->Corrupted;
peter>     }
peter>     while(length-- && !context->Corrupted)
peter>     {
context-> Message_Block[context->Message_Block_Index++] =
peter>                     (*message_array & 0xFF);

peter>     context-> Length_Low += 8;
peter>     if (context->Length_Low == 0)
peter>     {
peter>         context-> Length_High++;
peter>         if (context->Length_High == 0)
peter>         {
peter>             /* Message is too long */
peter>             context-> Corrupted = 1;
peter>         }
peter>     }

Why not add a longlong Length strucutre argument instead of 
having 'Length_Low' and 'Length_high' ?

<cut>

Otherwise ok;  Please fix the above and then push this into the 4.0
tree!

Regards,
Monty
Thread
bk commit into 4.1 treepeter10 May
  • bk commit into 4.1 treeMichael Widenius19 May