Hvala na odgovoru Jovane...;-) I don't see what the problem would be with skipping those two lines... They are not skipped if malloc() returns NULL, rather, they are skipped whenever the requested size is 0 (in which case malloc() itself is also skipped). clear() (which is called just before) already sets the data to NULL and size to 0 (which is what is being requested :-)). And, since we have not made a call to malloc() in the first place, we don't technically own anything (so we shouldn't be setting _owned to true)? Omitting a (relatively slow) kernel call (such as malloc()) when it is clearly unnecessary (client is asking for 0 bytes!) - seems like a sound practice (?)...
The only small issue I see with this is that clear() itself doesn't set _owned to false after a call to free() (which, it seems to me, it definitely should(!) - as the blob object doesn't own anything after a call to free())... I think I will make that change in my copy as well (but, of course, I won't be merging anything back as I am far too much of a noob with this library to dare to contribute)... :-) Regards, Marko On Wed, Dec 30, 2020 at 4:05 PM BJovke <bjo...@gmail.com> wrote: > Здраво Марко. :) > > This is the malloc() docs: > > *If size is zero, the behavior is implementation defined (null pointer may > be returned, or some non-null pointer may be returned that may not be used > to access storage, but has to be passed to std::free > <https://en.cppreference.com/w/cpp/memory/c/free>) * > So it might be non-NULL as well as NULL but obviously the code thinks it's > always not NULL if successful. > Your change is not good because it skips these lines in NULL case: > > _size = size_; > > _owned = true; > > I would change the assert to: > alloc_assert (_data != NULL || size_ == 0); > and skip only memcpy() call. > > There's also a malloc's matching free() but this will be OK because > calling free() on NULL is harmless. > > You need to check for potential other memcpy's, asserts or similar stuff > on this memory. > > Greetings! > > сре, 30. дец 2020. у 06:35 Marko Majic <mmaji...@gmail.com> је написао/ла: > >> Hi all, >> >> >> This is a repeat as I am not sure if my original message made it onto the >> mailing list (my apologies if it did!). >> >> >> I am trying to build a libzmq library (as a Windows DLL) with an >> unsupported compiler. I had to make a few changes to various source files >> (mostly having to do with the mismatched class method signatures as many >> declarations in header files seem to be dropping ‘const’ qualifiers from >> the actual definition in the source which, for some strange reason, works >> fine on my 64-bit compiler but results in missing linker symbols in my >> 32-bit compiler?). However, in the end (after those changes), it all >> compiled (and seemed) fine. >> >> >> >> I then wrote a small script to auto-execute all the test_*.exe programs >> after building is complete and (initially) most of them worked but any that >> didn’t would invariably fail with an ‘OUT OF MEMORY’ error pointing to >> blob.hpp. I found this odd – both because I have ample RAM on my dev box >> and because I didn’t think libzmq would be a particular memory hog – so I >> put a few diagnostic messages in blob.hpp set() method and it seems, that >> all the calls to set() that actually fail are requesting 0 bytes. This, of >> course, made sense – since malloc() would be returning NULL when 0 bytes >> are requested which, in turn, would trip the subsequent alloc_assert(); >> call (macro, that is). >> >> >> >> To get around this issue, I changed the set() code from: >> >> void set (const unsigned char *const data_, const size_t size_) >> >> { >> >> clear (); >> >> _data = static_cast<unsigned char *> (malloc (size_)); >> >> alloc_assert (_data); >> >> _size = size_; >> >> _owned = true; >> >> memcpy (_data, data_, size_); >> >> } >> >> >> >> To: >> >> void set (const unsigned char *const data_, const size_t size_) >> >> { >> >> clear (); >> >> if(size_) { >> >> <…same as above…> >> >> } >> >> } >> >> >> >> And also made the same change to the (equivalent) call set_deep_copy(). >> >> >> >> On the face of it, these changes make perfect logical sense (if making a >> copy of an empty blob – the result should be an empty blob, and when >> creating a blob of 0 bytes, the result should, again, be an empty blob). I >> re-compiled the library and test programs with the above changes to >> zmq::blob_t::set() and zmq::blob_t::set_deep_copy() and now all of the test >> programs execute fine and pass all the tests - 0 failures, 3 ignored tests >> – 2 having to do with ZMQ_MULTICAST_LOOP=false not working on Windows >> (marked as “TODO” so – evidently a known issue) and one having to do with >> TIPC not being available (which, again, makes sense since I am running this >> on Windows and {I think?} TIPC is a Linux thing)… >> >> >> >> So, it all seems great - however, since I am (admittedly) a complete noob >> on the internal workings of libzmq (and the above seemed a bit >> “elementary”) - I just wanted to check in with the “hive mind” of people >> that know far more about libzmq than me. Are the changes I made OK? Or >> would they cause other problems (because empty blobs or requesting 0 bytes >> SHOULD cause assert failures)? If latter, how do those test programs work >> on other platforms/compilers (I would assume that malloc(0) on any C >> compiler on any platform will return NULL just like it did for me – which, >> of course, would trip the alloc_assert macro). Or do I have a problem >> because some of the test programs are, in fact, requesting 0-byte blobs >> (when, in fact, none of them should)? >> >> >> >> Any enlightenment is (always) greatly appreciated! >> >> >> >> Cheers, >> >> >> >> Marko >> >> >> _______________________________________________ >> zeromq-dev mailing list >> zeromq-dev@lists.zeromq.org >> https://lists.zeromq.org/mailman/listinfo/zeromq-dev >> > > > -- > > Jovan Bunjevački. > _______________________________________________ > zeromq-dev mailing list > zeromq-dev@lists.zeromq.org > https://lists.zeromq.org/mailman/listinfo/zeromq-dev >
_______________________________________________ zeromq-dev mailing list zeromq-dev@lists.zeromq.org https://lists.zeromq.org/mailman/listinfo/zeromq-dev
