zeromq / libzmq

ZeroMQ core engine in C++, implements ZMTP/3.1
https://www.zeromq.org
Mozilla Public License 2.0
9.71k stars 2.35k forks source link

Fixes for memory allocation and zero size #4502

Closed casaroli closed 1 year ago

casaroli commented 1 year ago

Hello,

I am building ZeroMQ with NuttX and this is part of a probably series of minor PRs.

I see a scenario where size is 0, so we are getting a spurious out of memory error.

BR

bluca commented 1 year ago

memcpy with null src is undefined behaviour

bluca commented 1 year ago

Please add a relicense statement https://github.com/zeromq/libzmq/tree/master/RELICENSE

bluca commented 1 year ago

there are other cases in this class where there will zero-size and undefined pointer memcpys. It's probably easier to just fix wherever you see that it's doing zero-sized blobs?

casaroli commented 1 year ago

Please add a relicense statement https://github.com/zeromq/libzmq/tree/master/RELICENSE

done

casaroli commented 1 year ago

there are other cases in this class where there will zero-size and undefined pointer memcpys. It's probably easier to just fix wherever you see that it's doing zero-sized blobs?

I am in a parallel task trying to run the tests (on NuttX) so I will propose changes as I encounter them.

If you prefer I can have a look at this class and just add guards similar to this to all relevant places, but maybe it is cleaner to do this as I encounter problems.

WDYT?

bluca commented 1 year ago

Please fix them all at once, we cannot risk adding undefined behaviour

casaroli commented 1 year ago

Please fix them all at once, we cannot risk adding undefined behaviour

done

bluca commented 1 year ago

Please fix the formatting as the CI suggests

--- a/src/blob.hpp
+++ b/src/blob.hpp
@@ -92,8 +92,7 @@ struct blob_t
         _owned (true)
     {
         alloc_assert (!size_ || _data);
-        if (size_ && _data)
-        {
+        if (size_ && _data) {
             memcpy (_data, data_, size_);
         }
     }
@@ -132,8 +131,7 @@ struct blob_t
         alloc_assert (!other_._size || _data);
         _size = other_._size;
         _owned = true;
-        if (_size && _data)
-        {
+        if (_size && _data) {
             memcpy (_data, other_._data, _size);
         }
     }