Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:35311
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain openwall.com designates 195.42.179.200 as permitted sender)
Date: Thu, 7 Feb 2008 20:57:16 +0300
To: Dmitry Stogov <dmitry@zend.com>
Cc: PHP Internals List <internals@lists.php.net>
Message-ID: <20080207175716.GA22184@openwall.com>
References: <20071209010552.GA12561@openwall.com> <47A849D0.8050508@zend.com> <20080205235055.GA19309@openwall.com> <47AAD95A.8010109@zend.com> <20080207112109.GA30558@openwall.com> <20080207115027.GA30811@openwall.com> <20080207121353.GA11906@openwall.com> <47AB05CB.8010906@zend.com> <20080207135336.GA21239@openwall.com> <47AB33F0.6070309@zend.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <47AB33F0.6070309@zend.com>
User-Agent: Mutt/1.4.2.3i
Subject: Re: untie the MD4 code from MD5 & replace the MD5 code
From: solar@openwall.com (Solar Designer)

On Thu, Feb 07, 2008 at 07:38:08PM +0300, Dmitry Stogov wrote:
> According to algorithm "used" and "free" in PHP_MD5Final cannot be more 
> than 64, so I don't see any reason for unnecessary conversions. Looking 
> more careful I think they must be changed into php_uint32 in 
> PHP_MD5Update too.

You're correct, except that it is non-obvious which incurs more type
conversions and which of those conversions have a runtime cost and which
don't.  For example, "&ctx->buffer[used]" on a 64-bit system may have to
expand "used" to 64 bits for effective address calculation, which may or
may not cost (depends on properties of the architecture and the compiler).
Similarly, passing "free" as the size argument to memcpy() may have to
expand it to 64 bits to match the ABI and/or the prototype for memcpy().

OK, I've tried to count those likely conversions - and my conclusion is
that having "used" and "free" at size_t in both functions results in
fewer conversions (or even none) when size_t matches the pointer size
and php_uint32 doesn't.  Specifically, the only places where anything
(that touches these two variables) would need to be zero-extended are:

	used = saved_lo & 0x3f;
...
	used = ctx->lo & 0x3f;

In both cases, we have a bitwise AND anyway - so the compiler might
instead expand the constant to 64-bit (or use an instruction that
expands its immediate argument to 64 bits instead of to 32 bits).

Anyway, this is obviously not important, so I am fine with your latest
patch the way it is, if you prefer to have it that way.

Thanks,

Alexander