Hello Johannes,
Johannes Brunen wrote:
after spending some time I have found the problematic code. Actually, I
still do not know what is the real problem but let me explain first.
ah, I was writing a mail to you with some updates, but I had not been
able to find the problem.
I have learned that the '.osb' file format involves the following kind of
code:
1. On writing: OSGExceptionBinaryDataHandler.inl, function void
putValue(const Real32& value)
Real32 v = osgHostToNet<Real32>(value);
which melts down to (OSGBaseFunctions.inl)
template <> inline
OSG::Real32 osgSwapBytes<Real32>(OSG::Real32 src)
{
UInt8* pStart = reinterpret_cast<UInt8*>(&src);
UInt8* pEnd = reinterpret_cast<UInt8*>(&src) + sizeof(Real32);
std::reverse(pStart, pEnd);
return src;
}
I could verify that everything is fine to the point of the return call.
However, variable v is not correctly intialized afterwards.
oh, my favourite C++ programming rules, aliasing ;) My hope in writing
the above had been that due to the special casing of 'char' in the
aliasing rules the conversion would be safe :-/
2. On reading the function getValues(Real32* value, UInt32 size) throw
(ReadError) is called in the same source file as above. A similar function
call takes place in this file which is also unsecure (with respect to my
platform):
value[i] = osgNetToHost<Real32>(value[i])
In order to check it more thoroughly, I modified the code in
OSGExceptionBinaryDataHandler in the following way:
Wrpt. to 1)
// Real32 v = osgHostToNet<Real32>(value);
UInt8 buffer[sizeof(Real32)];
memcpy(buffer, reintepret_cast<const void*>(&value), sizeof(Real32));
std::reverse(buffer, buffer+sizeof(Real32));
put(buffer, sizeof(Real32));
Wrpt. 2)
//value[i] = osgNetToHost<Real32>(value[i]);
std::reverse(reinterpret_cast<UInt8*>(&value[i]),
reinterpret_cast<UInt8*>(&value[i])+sizeof(Real32));
With these small modifications, I'm not facing any of the reported problems
anymore.
Unfortunately, I'm not an expert in these matters and can't tell what the
real reason for the problem is. Interestingly, I added the following code
for testing:
Wrpt 1.
if (::boost::math::isnan(v))
__asm int 3;
Wrpt 2.
if (::boost::mathisnan(value[i])
__asm int 3;
These interrupts get be called in the original code. As I have told, I do
not know the exact rules about copying float (doubles) numbers in case of
leaving the valid range of numbers. What I know is that the math coprocessor
is always good for surprises especilly folded by the optimization flags of
the compiler.
hm, the tests are done after the swap, right? In that case it is ok that
the numbers are NaNs since they are only stored/send over the network in
this format. Before any computation is done on them they are converted back.
The whole conversion of floating point numbers to "network format" is a
bit dubious anyway, since there is not really a "network format" for
them in the first place, unlike for integers. But OpenSG has done these
conversions forever and changing it now would probably introduce
breakage of the .OSB format, which I'd rather avoid.
So, what to do. I would recommend to go for a little more secure way for the
osgHostToNet and osgNetToHost functions. At least on my platform (win32,
win64) I'm expecting more trouble in this area, so.
What do you think? Gerry, in case you are listening, do you know the
background for this stuff? What is your opinion?
I'll change the functions to use type punning through unions (AFAIK this
is also not allowed by the C++ standard, but a supported extensions on
GCC and MS VS) - patch attached. Unless someone has a better idea?
Sorry for the trouble and many thanks for tracking down the problem!
Cheers,
Carsten
diff --git a/Source/Base/Base/OSGBaseFunctions.inl b/Source/Base/Base/OSGBaseFunctions.inl
index c73f8ec..87ab6f0 100644
--- a/Source/Base/Base/OSGBaseFunctions.inl
+++ b/Source/Base/Base/OSGBaseFunctions.inl
@@ -2939,12 +2939,16 @@ OSG::Int64 osgSwapBytes<Int64>(OSG::Int64 src)
template <> inline
OSG::Real32 osgSwapBytes<Real32>(OSG::Real32 src)
{
- UInt8* pStart = reinterpret_cast<UInt8*>(&src);
- UInt8* pEnd = reinterpret_cast<UInt8*>(&src) + sizeof(Real32);
+ union
+ {
+ OSG::Real32 floatVal;
+ OSG::UInt32 intVal;
+ } unionVal;
- std::reverse(pStart, pEnd);
+ unionVal.floatVal = src;
+ unionVal.intVal = osgSwapBytes(unionVal.intVal);
- return src;
+ return unionVal.floatVal;
}
/*! \ingroup GrpBaseBaseMiscFn
@@ -2952,12 +2956,16 @@ OSG::Real32 osgSwapBytes<Real32>(OSG::Real32 src)
template <> inline
OSG::Real64 osgSwapBytes<Real64>(OSG::Real64 src)
{
- UInt8* pStart = reinterpret_cast<UInt8*>(&src);
- UInt8* pEnd = reinterpret_cast<UInt8*>(&src) + sizeof(Real64);
+ union
+ {
+ OSG::Real64 floatVal;
+ OSG::UInt64 intVal;
+ } unionVal;
- std::reverse(pStart, pEnd);
+ unionVal.floatVal = src;
+ unionVal.intVal = osgSwapBytes(unionVal.intVal);
- return src;
+ return unionVal.floatVal;
}
/*! Convert a value from host byte order to big endian byte order.
------------------------------------------------------------------------------
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://ad.doubleclick.net/clk;226879339;13503038;l?
http://clk.atdmt.com/CRS/go/247765532/direct/01/
_______________________________________________
Opensg-users mailing list
Opensg-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/opensg-users
