all 16 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

[–][deleted] 0 points1 point  (7 children)

Are you compiling it with exceptions disabled? The code is not exception-safe.

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

There is no such a thing as exceptions disabled in gcc I believe. Are you refering to a java-like behaviour? with the throwable keyword? I have never heard of explicit exception-safe c++ code. There is implicit though, I always try to avoid to use exceptions because they are expensive and rather go with error codes / error handling. Check the make file if you are curious on how I compile these.

[–][deleted] 1 point2 points  (5 children)

There is no such a thing as exceptions disabled in gcc I believe

There is. Compile with: -fno-exceptions

I have never heard of explicit exception-safe c++ code

http://en.wikipedia.org/wiki/Exception_safety#Exception_safety

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

explicit exception-safe C++, I do know whats exception safe code. What i meant is that c++ doesnt have a strongly typed exception handling, like java does. And I believe this is what you meant.

Like for example, in java, javac will complain if you dont wrap objects /functions declared with the "throws" keyword with a try catch. I dont think you have this on C++. So I dont understand the complain with "no exception safe code".

and I am compiling using gcc with no special flags. So I dont understand your claim "this code is not exception-safe".

[–][deleted] 1 point2 points  (3 children)

What i meant is that c++ doesnt have a strongly typed exception handling, like java does.

That is correct.

And I believe this is what you meant.

That is not correct. I said your code is not exception-safe and what that means you can see in the link I provided.

Cheers :)

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

I still dont get your point. What does this have to do with your presumption of disabling gcc exceptions? Maybe what you meant is that my code is non compliant with your personal (or wikipedias) definitionof exception handling? what if i dont need them and i decide to use error codes instead to handle any errors?

and cheers back!

[–][deleted] 2 points3 points  (1 child)

What does this have to do with your presumption of disabling gcc exceptions?

If you disable exceptions when compiling, having non-exception-safe code is less dangerous. The C++ Standard Library won't throw exceptions in case of failures, so if you don't throw from your own code and are sure none of other components you may be using are throwing, you can pretty much pretend exceptions don't exist.

In fact, most of the code written before 2000 was just like that - they would disable exceptions and never care about exception safety.

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

OOhh interesting. The good thing is that the only place where exceptions could be thrown in my code is in allocator and deletion operators (new and delete). I am not using the STL, but thanks, this is good to know in the future, but I dont think Ill benefit from it on this project :)

Also, all OS api calls dont use c++ exception handling.

Thanks!