[PATCH 4 of 6 v6 lazy-manifest] manifest: use custom C implementation of lazymanifest

Martin von Zweigbergk martinvonz at google.com
Thu Mar 12 11:15:52 CDT 2015


I'll send a patch soon. Matt, let me know if it helps you.

On Thu, Mar 12, 2015 at 8:33 AM Martin von Zweigbergk <martinvonz at google.com>
wrote:

> On Wed, Mar 11, 2015 at 10:53 PM Martin von Zweigbergk <
> martinvonz at google.com> wrote:
>
>> On Wed, Mar 11, 2015 at 9:26 PM Matt Harbison <mharbison72 at gmail.com>
>> wrote:
>>
>>> I managed to get a debug version built with various _ASSERTE(
>>> _CrtCheckMemory( ) ) calls sprinkled around.  It asserts in
>>> lazymanifest_setitem() after calling 'sprintf(dest + plen + 1 + (i * 2),
>>> "%02hhx", hash[i]);'.
>>>
>>> I don't see an 'hh' size prefix [1], but dropping one of the 'h' didn't
>>> change anything.  I got this from DebugView, tracing the relevant
>>> variables:
>>>
>>
>> It seems unclear what 'hh' really means, see [1].
>>
>> The '2' for the 'width' parameter is a *minimum*. I suppose if the hash
>> was negative, it might get sign-extended and then printed as 8 hex
>> characters? Does casting "hash[i]" to "unsigned char" help? But maybe it's
>> better to use snprintf?
>>
>> Also, if the hash byte (char) get sign-extended, it seems like that would
>> mean that any negative byte would be result in "ff", because "ffffffxx"
>> gets written and most of it gets overwritten.
>>
>> While writing the above, I thought that C's "char" was always signed (and
>> had always thought so), but according to [2], it's implementation-defined
>> whether it's signed or unsigned. It seems like most people's
>> implementations use unsigned chars, while yours uses signed chars. I
>> suppose that means that when your version does succeed without crashing,
>> your repo is likely to not verify, since the hash will be messed up. Does
>> it sometimes not crash? Can you check whether 'hg verify' works after
>> making a commit or two?
>>
>> [1] http://stackoverflow.com/questions/4586962/what-is-the-p
>> urpose-of-the-h-and-hh-modifiers-for-printf.
>> [2] http://stackoverflow.com/questions/2054939/is-char-signed-
>> or-unsigned-by-default
>>
>>
> It turns out char is signed on my system too, so I suppose what saves me
> is that my sprintf does treat 'hh' by treating the parameter as char width,
> while it seems like your sprintf doesn't. So, I think dropping the "hh" and
> sticking a cast to unsigned char in there is a good solution. Augie, what
> do you think?
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150312/f877db1a/attachment.html>


More information about the Mercurial-devel mailing list