search

ros / roscpp_core

ros distribution sandbox
89 stars 116 forks source link

Conversion from size_t to uint32_t in vector serialization #126

Closed csorvagep closed 1 year ago

csorvagep commented 3 years ago

In roscpp_serialization/include/ros/serialization.h:417 v.size() returns with size_t and the serializedLength returns with uint32_t. size_t on 64-bit systems usually 64 bit wide, so this line cause a warning. (I noticed this on windows, where some warnings are enabled by default.)

The proper return type should be size_t, but since size_of(T) is already converted to uint32_t, this can be fixed with another static cast.

I can create a pull request if needed.

peci1 commented 1 year ago

Just adding a static cast does not make things much better. It would silence the warnings, but the logic would remain broken. A proper solution would need to check if the resulting number will fit into uint32_t and throw an exception if not.

Would you be able to provide a PR with such fix?

csorvagep commented 1 year ago

I think I meant to cast the uint32_t value to size_t, which is an upcast, and could solve the problem. But to be honest, this was more then two years ago, I have no recollection to what was the actual problem and nor do I have the sources and tools installed. I was definitely ready to create a PR back then, but now equally not. so I'll leave this to someone else. Or just close this issue.

peci1 commented 1 year ago

Ah, right, upcast should be okay then. Since this should have no functional impact and nobody else cares about the warning, I'm closing this issue.

Anybody, feel free to comment if it still is an issue for you.