I first saw the following coding problems when using 0.9.6i, but they are
still there in the current 0.9.7 snapshot.  I'm using Microsoft Visual
Studio 6 but these problems are not related to the compiler:
 
(1) crypto\asn1\a_mbstr.c, function ASN1_mbstring_ncopy() -- local variable
'outlen' is not initialized because 'switch(outform)' does not have a
default case.  Suggestion:  add 'default: outlen = 0;' to switch statement.
 
(2) crypto\asn1\a_strex.c, function do_buf() -- local variable 'c' is not
initialized because 'switch(type & BUF_TYPE_WIDTH_MASK)' does not have a
default case.  Suggestion:  add 'default: c = 0;' to switch statement.
 
(3) crypto\ans1\asn1_mac.h, macro M_ASN1_D2I_vars -- the local variable 'c'
is not fully initialized.  Suggestion:  add 'memset(&c, 0, sizeof(c));' to
the macro.
 
(4) crypto\ans1\i2d_dhp.c, function i2d_DHparams() -- the local variable '
p' is not initialized.  Suggestion:  add 'p = NULL;' to the beginning of the
function.  (This applies to 0.9.6 only as the file is deleted in 0.9.7)
 
(5) ssl\s23_srvr.c, function ssl23_get_client_hello() -- the declaration of
the local variable 'use_sslv2_strong' should be enclosed with '#ifndef
OPENSSL_NO_SSL2'; otherwise it is an unused variable.
 
(6) crypto\pkcs7\pk7_enc.c -- the prototype 'PKCS7_add_signer(PKCS7 *p7,X509
*cert,EVP_PKEY *key);' does not match the actual definition in
crypto\pkcs7\pk7_lib.c, which is 'int PKCS7_add_signer(PKCS7 *p7,
PKCS7_SIGNER_INFO *psi)'.  Suggestion:  remove prototype since it is not
used within pk7_enc.c anyway.
 
(7) crypto\threads\th-lock.c, function CRYPTO_thread_setup() -- the
statement 'CRYPTO_set_locking_callback((void (*)(int,int,char
*,int))win32_locking_callback);' is missing a 'const' in the casting.  It
should be:   'CRYPTO_set_locking_callback((void (*)(int,int,const char
*,int))win32_locking_callback);'
 
(8) crypto\threads\th-lock.c, function CRYPTO_thread_setup() -- should not
'return(1);' since this is a void function.
 
(9) apps\req.c, function MAIN() -- after the statement
'BIO_printf(bio_err,"Unable to load config info\n");', there should be a
'goto end;' to prevent access of the null variable 'req_conf' later in the
function which may crash the system.
 
(10) crypto\bio\bss_acpt.c, function acpt_close_socket() -- 'shutdown(, 2)'
should be replaced with 'SHUTDOWN2' for portability.
 
(11) crypto\bio\bss_conn.c, function  conn_close_socket() -- 'shutdown(, 2)'
should be replaced with 'SHUTDOWN2' for portability.
 
(12) ssl\s23_meth.c(66), function ssl23_get_method() -- suggest to wrap 'if
(ver == SSL2_VERSION) return(SSLv2_method()); else' with '#ifndef
OPENSSL_NO_SSL2' for consistency. and to reduce linking in stuff
unnecessarily.
 
(13) ssl\ssl_sess.c, function timeout() -- may I humbly suggest that this
function be renamed uniquely, such as SESSION_timeout() to minimize symbol
clash.
 
Thank you for your considerations.
 
Henry Luis
Echelon Corporation
 
 
______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       [EMAIL PROTECTED]
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to