|
declaring it on the stack? ouch.
I only return char*s from functions if I'm using some kind of memory management scheme that allows for it. Unless you implement one C and C++ ... doesn't.
I can't judge people too badly though, considering I just got schooled on using leading underscores in my local member names (a habit I picked up from C#)
But still, you should understand scoping if you're going to get paid to code in the thing.
Real programmers use butterflies
|
|
|
|
|
Yes, in C#, we (used to) use underscores for local member names. In C++, we used 'm_' for member names. The 'm' told us that it was a class (or struct) member.
|
|
|
|
|
i should have known. i went back to that style and I absolutely hate it.
Real programmers use butterflies
|
|
|
|
|
One of the problems with stack allocation is on platforms like the ESP32 and most of the arduinos, they don't give you a lot of stack space. I know usually one grows up and the other grows down but I run out of stack declaring 2kB blocks sometimes so there might be some kind of artificial limit.
That's why I usually use the heap for my pools.
Real programmers use butterflies
|
|
|
|
|
How much can you sell it for? And/or can you get 1 million (or more) devs to use it?
That's our modern (cynical) measure for success so get on board.
|
|
|
|
|
I gave it away under the MIT license. It was just a little bit of code anyway.
Real programmers use butterflies
|
|
|
|
|
honey the codewitch wrote: I gave it away under the MIT license.
GitHub link, or it didn't happen.
|
|
|
|
|
|
I see you're using both #pragma once and #ifdef include guards. Is that really necessary? GCC supports both, going back to at least version 4.8, so the #pragma doesn't even need to be wrapped in an #ifdef _MSC_VER. But maybe you know something I don't, or maybe you're using some other compiler that doesn't understand the #pragma?
Also, picking nits, since I have nothing better to offer, I see that this is a memory pool for contrained memory environments. That must be a constrained, contained memory situation, correct?
Keep Calm and Carry On
|
|
|
|
|
I used to put a test of the preprocessor in headers :
#pragma once
#ifdef HEADER_H
#error pragma once was ignored
#else
#define HEADER_H
#endif I have not used very many compilers but I never saw the error message. I use only VS now so I don't do that any more.
"They have a consciousness, they have a life, they have a soul! Damn you! Let the rabbits wear glasses! Save our brothers! Can I get an amen?"
|
|
|
|
|
I could do that but I'd rather recover gracefully. What I did compiles with no warnings and that's my major concern.
Real programmers use butterflies
|
|
|
|
|
GCC throws a "#pragma once in main file" if I don't guard against them appearing.
OTOH MSVC seems to like #pragma once to be in there for its source editors.
Real programmers use butterflies
|
|
|
|
|
|
I'm still kind of massaging the code. It's so simple it works as is and will probably remain basically the same, but I've tweaked little things like changed the template parameter from C to TCapacity
I'm still not sure I like so I haven't recommitted yet but any changes will be minor.
Real programmers use butterflies
|
|
|
|
|
Would you mind if I gave you a small critique on that code?
|
|
|
|
|
go ahead. my C++ is rusty so I'm sure there's stuff to be improved.
Real programmers use butterflies
|
|
|
|
|
Not much improvement, just a few observations:
1. Identifiers starting with underscores are reserved. If you use them then your program is non-conforming for no good reason.
2. The comparison against capacity in both the static and dynamic classes result in never being able to use the last byte of the pool: The "used()>=capacity" should be "used()>capacity". To test it instantiate a pool of 10 bytes and allocate 6. The (capacity() - used()) is then 4, but a further allocation of 4 fails. A further allocation of 3, on the other hand, succeeds and (capacity() - used()) is then 1.
3. The static pool could benefit from a #warning directive when C is too large. Right now a 8MB C when instantiating it (1024 * 1024 * 8) would almost certainly overflow the stack, and 8MB is not a lot of memory.
|
|
|
|
|
1. I thought they were only reserved for globals. I stand corrected.
2. Good catch.
3. The static pool can and often is declared as a global, making it heap/not stack, which is where it's primarily designed to go. DynamicMemoryPool is probably a better choice if you need a locally scoped pool because it always allocates from the heap.
Thanks!
Real programmers use butterflies
modified 15-Dec-20 8:36am.
|
|
|
|
|
1) gotta love C++, since rules for reserved names are even more complex that, depending on scope, case, number of underscores...
3) or static_assert to keep it in the family language.
|
|
|
|
|
1) ditch the using namespace std; in header files,
2) you don't have virtual destructor for MemoryPool and
3) if you're using C++17 you might want to check memory_resource class/header
|
|
|
|
|
1) forgive me for asking, but why? edit: whoops that was an error. if anything it was supposed to be inside the file's namespace
2) MemoryPool is an interface - a pure abstract base. what is the purpose of a virtual destructor in such a contract as it holds no resources? - never mind. I was thinking about the call chain backwards. derived classes need to have their destructor called if the base goes out of scope. i forgot. I'm rusty.
3) I'm targeting C++11 for now because reasons having to do with the platforms this is primarily for.
Real programmers use butterflies
modified 15-Dec-20 20:50pm.
|
|
|
|
|
1) you don't want to introduce bunch of names from std into client's scope, can cause all kind of nasty problems for users. C++ name lookup is complex as it is.
2) if I got a pointer to MemoryPool and tried to delete the referenced object, I would invoke undefined behavior, even thought virtual methods are strongly suggesting me that I should be able to do it
Some more points, since you said C++ is love:
4) virtual void* alloc(const size_t size)=0; - const is needless
5) if(!TCapacity) will give you a warning (on /W4 maybe) if TCapacity is 0, but
6) the bigger problem is uint8_t m_heap[TCapacity] , since zero-sized arrays is not standard C++
So I would either go with static_assert and ensure that 0 is not valid value or make specialization for that cas.
|
|
|
|
|
1&2 - see my edited post
4, i use const descriptively, even if it doesn't do anything, unless there's a reason I shouldn't
5, yeah that's a nasty habit of mine
6 I introduced a static_assert to fix that in my later code.
Real programmers use butterflies
|
|
|
|
|
honey the codewitch wrote: 4, i use const descriptively, even if it doesn't do anything, unless there's a reason I shouldn't
To whom this description is directed? Users of the interface don't not care about it. Implementer of the interface is free to leave it out when override the function, since compiler ignores const when doing overload resolution. It is just implementation thing leaking into syntax of function signature, but not really affecting it. You can keep it class implementing abstract function to prevent changes to the parameter in the function's body, but in the base class it just confusing.
|
|
|
|
|
And some more:
7) You invoke undefined behavior in ~DynamicMemoryPool by calling delete operator instead of delete[]
8) i guess capacity , used , next should be const -qualified
C++ IS LOVE
|
|
|
|