mjung at iss.tu-darmstadt.de
Tue Oct 26 03:13:59 CDT 2004
Hello Juan, thanks for your comments!
> 1. 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
> 2. 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.
> 3. Get rid of magic numbers: What do the values 0x36
> and 0x5c mean in copy_hmac_info? What is the length
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
> 5. In some of the files you have the incorrect
> filename in the comments, e.g. dlls/rsabase/handle.c,
Thanks, I'll fix those.
> 6. 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.
More information about the wine-devel