You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While investigating #849, I noticed that there are several places in YARP where size_t (usually 64 bit on 64 bit machine) are implicitly converted to int (32 bit). Besides risking to alter the value of the variable, this generates many warnings on windows, and, if -Wconversion is enabled, also on linux.
For example when a string is sent through the network, YARP sends the length of the string as an int, and then the string itself. The same happens for blobs, vectors, and similar. This may alter a value for very long strings, etc.
On the other hand, a very long string sent from a 64 bit machine could be impossible to store for 32 bit machine.
These are the options that I see:
Forget about it and leave everything as it is right now.
Forget about it and leave everything as it is right now, but suppress the warnings.
Change the protocol to send the size always as a 64 bit variable (both on 32 and 64 bit) instead of a 32 bit one. This would make YARP incompatible with previous versions. Moreover there will be a 64 to 32 bit conversion from the received int64 value to size_t on 32 bit machines that could actually cause issues if the sender is 64 bit.
Extend the protocol to send 64 bit version of string, blob, vector, etc. I don't see a really advantage in doing this, though.
Explicitly limit the size of this kind of variables to 32 bit.
After a few tests for 3. and 4., I'd rather go for 5. since it is the only one that will work without issues on 32 bit and will keep the compatibility with previous versions. Anyway the warnings are really annoying, and moreover the value could actually be altered, therefore I suggest to:
Compare the size with INT_MAX before sending it and eventually print a warning and truncate the string/blob/vector. This will add an extra check almost every time that something is sent, I'm not sure if this will have a big impact on the performances.
Use explicit c++ casts instead of c-style casts that will suppress the warnings.
This discussion was converted from issue #859 on December 22, 2020 09:08.
Heading
Bold
Italic
Quote
Code
Link
Numbered list
Unordered list
Task list
Attach files
Mention
Reference
Menu
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
While investigating #849, I noticed that there are several places in YARP where
size_t
(usually 64 bit on 64 bit machine) are implicitly converted toint
(32 bit). Besides risking to alter the value of the variable, this generates many warnings on windows, and, if-Wconversion
is enabled, also on linux.For example when a string is sent through the network, YARP sends the length of the string as an
int
, and then the string itself. The same happens for blobs, vectors, and similar. This may alter a value for very long strings, etc.On the other hand, a very long string sent from a 64 bit machine could be impossible to store for 32 bit machine.
These are the options that I see:
After a few tests for
3.
and4.
, I'd rather go for5.
since it is the only one that will work without issues on 32 bit and will keep the compatibility with previous versions. Anyway the warnings are really annoying, and moreover the value could actually be altered, therefore I suggest to:INT_MAX
before sending it and eventually print a warning and truncate the string/blob/vector. This will add an extra check almost every time that something is sent, I'm not sure if this will have a big impact on the performances.Comments?
CC: @lornat75
Beta Was this translation helpful? Give feedback.
All reactions