Hello Juan, thanks for your comments!
- MD4, MD5, and SHA1 are now implemented in wine's
advapi32. Please use these instead of OpenSSL's.
Yes, that makes sense. Would it be possible to move the SHA1, MD4 and MD5 related code to some other directory in the wine tree and link to it statically from both advapi32 and rsaenh? Is the misc directory meant for this purpose?
- The regression tests should be written so they
don't fail if OpenSSL isn't available.
Yes, the regression tests definitely need some code cleanup.
- Get rid of magic numbers: What do the values 0x36
and 0x5c mean in copy_hmac_info? What is the length 64?
Those are actually part of the HMAC algorithm and don't have meaningful names. I will add references to an HMAC paper in a comment. Do you think it makes sense to call them HMAC_DEFAULT_36 and HMAC_DEFAUL_5C or something?
Finally, some stylistic nits: 4. Please break long lines.
Is there a rule for the maximum line lenght in wine? Would 128 characters be ok?
- In some of the files you have the incorrect
filename in the comments, e.g. dlls/rsabase/handle.c, dlls/rsabse/handle.h
Thanks, I'll fix those.
- Your indentation in some places is inconsistent;
you mix spaces and tabs, and that makes it hard for me to guess the appropriate tab size.
Are tabs or spaces prefered in wine? Is there a common tabwidth? How to I get vi to store tabs as white spaces?
Is it ok to post a 30kB patch to wine-devel or is it preferable to put it on the web and just post a link?
Thanks again for taking the time to review my patch.
Greetings, Michael