Re: Crypto Review

[Vladimir 'phcoder' Serbinenko]

>+#define MAX(a, b)      (a > b ? a : b)
Dangerous. Use ((a)>(b)?(a):(b))
>+               grub_printf("%c%c", hexchars[(buffer[i]>>4)&0xF], 
>hexchars[buffer[i]&0xF] );
Use %02x instead of ad-hoc code. Where is print_buffer used at all?
Perhaps grub_printf or grub_dprintf will be better?
>+struct cipher_list
>+{
>+  grub_cipher_t cipher;
>+  struct cipher_list *next;
>+};
For the reasons of compactness can you reuse list functions from list.h?
>+  if (cipher == NULL)
>+    {
>+      const char *errstr = "Illegal cipher type";
You can use the construction like
switch ()
case ...:
 return ...;
case  ...:
  return ...;
which is more readable imho
Can you move 'hash' command to a separate module (it's not useful to
embed in core.img in many cases)
Cipher interface looks similar to libgcrypt but not hashes. Is it
possible to have an interface similar to libgcrypt? We don't have to
replicate the interface. libgcrypt has goals of speed and we have
goals of size but if libgcrypt-like interface is as small as custom
one I would prefer libgcrypt-like for maintainance reasons.
> --
> http://michael.gorven.za.net
> PGP Key ID 1E016BE8
> S/MIME Key ID AAF09E0E
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git