Fwd: Comment on revision r4470 in tinymux

[Stephen Dennis]

Previous code review comments.

---------- Forwarded message ----------
From: <tin...@googlecode.com>
Date: Fri, Jan 15, 2010 at 11:44 AM
Subject: Comment on revision r4470 in tinymux
To: brazi...@gmail.com


brazilofmux commented on revision r4470 in project tinymux.
Details are at http://code.google.com/p/tinymux/source/detail?r=4470


Line-by-line comments:

File: /branches/dev_brazil/mux/src/functions.cpp (r4470)
===============================================================================

Line 10887:     {T("DIGEST"),      fun_digest,           2, 1,       2,         0, CA_PUBLIC},
-------------------------------------------------------------------------------
To be compatible with PennMUSH, this is the one case in the function table where this field is neither 1 nor MAX_ARG. It causes commas after the first to be treated as part of the argument which may confuse users. For example,

digest(sha1,abc,def) will calculate a SHA-1 digest of "abc,def" not "abcdef".

File: /branches/dev_brazil/mux/src/funmath.cpp (r4470)
===============================================================================

Line 2695:
-------------------------------------------------------------------------------
buf is not tested for NULL.

Line 2697:     const UTF8 digits[] = "0123456789ABCDEF";
-------------------------------------------------------------------------------
This string also appears in mathutil.cpp for use by mux_utox(). While the string is short, and the compiler is smart, it might be useful to combine the two.

Line 2746:     const EVP_MD *mp = EVP_get_digestbyname((const char *)fargs[0]);
-------------------------------------------------------------------------------
What is the potential mischief from passing fargs[0] (which will contain UTF-8) into a library routine doing string comparisons? Should this be validated before being passed down?

Secondly, is this case insensitive, and if so, should the strcmp() in the !SSL_ENABLED case be case insensitive?

Respond to these comments at http://code.google.com/p/tinymux/source/detail?r=4470
--
You received this message because you starred this review, or because
your project has directed all notifications to a mailing list that you
subscribe to.
You may adjust your review notification preferences at:
http://code.google.com/hosting/settings