On Tue, Jul 14, 2015 at 6:20 PM, Robert Roessler <[email protected]> wrote:
> Jeffrey Walton wrote:
>>
>> ...
>> OK, the additional warnings on Visual Studio likely came because of
>>
>> https://github.com/weidai11/cryptopp/commit/e67480dd9e8dcda57939fc7480e273b8d2e81335.
>>
>> We also cleared some more misc.h warnings at
>>
>> https://github.com/weidai11/cryptopp/commit/828c550389f79b035f4518a79b7138d281f0961c.
>>
>>> believe the actual complaint from VS (2013 for me) is caused by the
>>> various
>>> low-level workhorse "rotate" functions declaring their "shift" param as
>>> an
>>> unsigned char - which is where the actual alleged truncation is taking
>>> place
>>> (or would).
>>
>>
>> Here's the commit/comment on the truncation to char:
>>
>> https://github.com/weidai11/cryptopp/commit/457eaeaf2317faa30c86d6d3fc289f40c58cf177.
>> It added assert(y <= 255), which turned out to be spurious because of
>> assert(y <= sizeof(T)*8). The latter was really assert(y <= 32).
>>
>> But I'm having trouble parsing what the problem is, or what you want me to
>> do.
>>
>> Where do you see a problem, or what do you suggest we do?
>
>
> I obviously left out some details... like what exactly I was talking about.
> ;)
>
> I get the C4242 issues (in the MSVC conditional block between line 752 and
> line 823) on all the "shift and rotate" functions in misc.h because the
> underlying workhorse functions being called expect an "unsigned char" for
> the shift/rotate count param, but the wrapper functions themselves are
> specifying an "unsigned int" param... so when it is passed down to the inner
> funcs, it generates the warning.
>
> So, for these, just use whatever form of cast you like the best to say "hey,
> these are really unsigned chars" on the shift/rotate count param when you
> make the inner calls. :)
Here's one of the functions (from line 755):
template<> inline
word16 rotlFixed<word16>(word16 x, unsigned int y)
{
assert(y < 8*sizeof(x));
return static_cast<word16>(y ? _rotl16(x, y) : x);
}
OK, so there are two use cases here. First is the library calling it
(1). Second is external callers using it (2).
*If* only use case (1) existed, then we could cast to silence it. But
because of use case (2), we want callers to have the benefit of
analysis.
We can possibly do something like: find all library callers (use case
(1)), and ensure they don't trigger the warning. Then, any warnings
that remain are user's code (use case (2)).
I think we really need a rotate that operates on a data type larger
than a byte. We can possibly do that with inline assembly on x86. But
we won't really have a solution for x64 unless we provide MASM64
routines in a stand alone source file (x64 lacks inline assembly).
> Further, I might be inclined to also roll back the commit that added all the
> static_cast<word16> stuff to this block - at least if it was motivated by
> thinking that was the cause of the C4242 warnings - since the type being
> returned by the wrapped functions is already compatible with the the types
> returned by the wrapper functions.
I think that was a different complaint. I think we are getting them
piecemeal because of the 'pragam once'. The idea was to avoid flooding
the code with all the warnings.
I'll verify it once I get a Windows VM fired up.
Jeff
--
--
You received this message because you are subscribed to the "Crypto++ Users"
Google Group.
To unsubscribe, send an email to [email protected].
More information about Crypto++ and this group is available at
http://www.cryptopp.com.
---
You received this message because you are subscribed to the Google Groups
"Crypto++ Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
