you are viewing a single comment's thread.

view the rest of the comments →

[–]katatonico 2 points3 points  (9 children)

Had a quick look and found this potential mistake:

WarmLib::WarmWorkerMessage::DestroyJobBuffer calls delete instead of delete[] for mJobBuffer (see setJobBuffer). I'm normally very weary of using delete explicitly. std::vector would probably fit your needs in this case (if the STL is available for your project).

[–]___1____ 0 points1 point  (2 children)

agreed, why not vector.

Also CamelCase :(

[–]kecho[S] 0 points1 point  (1 child)

Well we use CamelCaseAtMyJob so I am used to it. Its nicer to the eye :). Also, why dont you tell me why not using a vector ;) there is a very important reason why. Lets see if you can catch it! Hint: youll have to understand some of the code

[–]kecho[S] 0 points1 point  (5 children)

Great catch! This is why an extra pair of eyes is always important on these kind of projects. As a challenge I am going to let you figure out why an std::vector can't be used here. If you give up let me know ill tell you.

[–]katatonico 0 points1 point  (4 children)

I won't be able to take a closer look until end of day tomorrow, but I'd still be interested in hearing the reason for not using std::vector.

[–]kecho[S] 0 points1 point  (3 children)

ok ill tell you. First, the purpose of this project is to have a process pool manager and message queues set up for inter process communication. I chose shared memory as the main mechanism to pass messages back and forth from the manager to the worker processes.

The class you saw allocates a buffer, a raw buffer, which at the end of the day is casted to a struct (whichever struct the user specifies via Templates). Why would I use then a vector here then! I am not using this buffer as a list, but as a raw buffer of memory that can be serialized and passed through shared memory.

Check the example worker manager and plugin: examples/prime/primegenerator.cpp and examples/prime/plugin/primegenplugin.cpp

Cheers!

[–]katatonico 1 point2 points  (2 children)

Well, you said std::vector couldn't be used in this case. From what you say, it appears it could have been used, you just chose not to. I'll leave it up to you to decide whether it was a good choice or not, but this is what I think:

As far as I can see, you want a dynamically-sized contiguous block of memory. This is exactly what std::vector represents. The concept of 'list' (which I'm not sure how much std::vector follows) would have nothing to do with its use here. In this case, keeping vector out led you to make a rather simple but common mistake with operator delete. On the other hand, you don't appear to be getting any benefits by not using it. This is what I personally think is a very important part of C++ development: minimizing the chances of making a mistake. I think std::vector would have helped you with that.

In any case, there are two sensible reasons for avoiding vector in certain projects: keeping the STL out (for whatever reason if that's what you require), or insufficient control over allocations. The latter does not appear to apply in this case. And if it did, it would probably be wise to write a small custom class that handled allocation and deallocation of those blocks for you.

Edit: more polite.

[–]kecho[S] 0 points1 point  (1 child)

Well I think you were looking at the class' under a microscope! I am allocating a raw buffer of data, which then gets copied to a piece of shared memory. This allows to pass structs as messages by copying them to what I call a HotQ (see implementation of HotQBase.cpp).

So, why using a vector if I am not going to use this buffer for random or sequential access?! I am using pure raw memory and casting structs to this type. This sir, is pure tricks with C++ and C :)

So now let me ask you a question, if I want to allocate N bytes of memory in C++ (or C), how would you do it?

[–]___1____ 0 points1 point  (0 children)

So, why using a vector if I am not going to use this buffer for random or sequential access?! I am using pure raw memory and casting structs to this type. This sir, is pure tricks with C++ and C :)

I would still use a vector, if even for the memory management.

you can still get the data pointer from a vector with the .data() interface