Jump to content

  • Log In with Google      Sign In   
  • Create Account


#ActualKhatharr

Posted 31 December 2012 - 11:26 AM

allocate() calls _resize() in a way that basically boils down to:
size_t currentByteLength = (mUsedSize * mOffset);
size_t byteLengthOfNewData = (mOffset * n);
size_t resizeArg = currentByteLength + (byteLengthOfNewData * 2);
_resize(resizeArg);

But _resize() begins with:
size_t newSize = size;
    char* newArr = new (std::nothrow) char[newSize * mOffset]; //multiplying by mOffset again
    //there's no check of (newArr == NULL)

_resize() does not destroy the contents of mFreeList before attempting to use the C&P code from the ctor to reset it.

//This shouldn't happen, since .....
Then it's an error condition and should be handled as such.

Is this subtraction correct?
inline const AllocTypePtr getPtr(unsigned i) const{
  return reinterpret_cast<AllocTypePtr>(reinterpret_cast<char*>(mAllocList.mFirst)
    - (mOffset * i) + sizeof(MemChunk));}

Also, what is the purpose of argument 'n' in allocate()? It's used in the _resize() check but not after that.

Unless I'm quite more confused than usual this means that you have a large block of contiguous memory which contains MemChunk nodes interpolated with single data payloads. If you add the payload to the MemChunk then you can just cast the memory as a MemChunk pointer and use array indexing to fetch nodes. If you plan to have dynamic sizing using 'n' then I don't think you can set up all your nodes during allocation, since you don't know how much space is needed between them. (I don't understand why that's being done anyway.)

(OMG this forum bug... TO THE MOON, ALICE!)

#25Khatharr

Posted 31 December 2012 - 11:24 AM

allocate() calls _resize() in a way that basically boils down to:<pre class="_prettyXprint _lang-auto _linenums:1">size_t currentByteLength = (mUsedSize * mOffset);
size_t byteLengthOfNewData = (mOffset * n);
size_t resizeArg = currentByteLength + (byteLengthOfNewData * 2);
_resize(resizeArg);</pre><br />But _resize() begins with:<pre class="_prettyXprint _lang-auto _linenums:1">size_t newSize = size;
char* newArr = new (std::nothrow) char[newSize * mOffset]; //multiplying by mOffset again
//there's no check of (newArr == NULL)</pre>&nbsp;<br /><br />_resize() does not destroy the contents of mFreeList before attempting to use the C&amp;P code from the ctor to reset it.<br />&nbsp;<br />&nbsp;<br />&nbsp;<pre class="_prettyXprint _lang-auto _linenums:1">//This shouldn't happen, since .....</pre>Then it's an error condition and should be handled as such.<br /><br /><br />Is this subtraction correct?<br />&nbsp;<br />&nbsp;<pre class="_prettyXprint _lang-auto _linenums:1">inline const AllocTypePtr getPtr(unsigned i) const{
return reinterpret_cast&lt;AllocTypePtr&gt;(reinterpret_cast&lt;char*&gt;(mAllocList.mFirst)
- (mOffset * i) + sizeof(MemChunk));}</pre>&nbsp;<br /><br />Also, what is the purpose of argument 'n' in allocate()? It's used in the _resize() check but not after that.<br /><br />Unless I'm quite more confused than usual this means that you have a large block of contiguous memory which contains MemChunk nodes interpolated with single data payloads. If you add the payload to the MemChunk then you can just cast the memory as a MemChunk pointer and use array indexing to fetch nodes. If you plan to have dynamic sizing using 'n' then I don't think you can set up all your nodes during allocation, since you don't know how much space is needed between them. (I don't understand why that's being done anyway.)<br /><br />(OMG this forum bug... TO THE MOON, ALICE!)

#24Khatharr

Posted 31 December 2012 - 11:16 AM

allocate() calls _resize() in a way that basically boils down to:
size_t currentByteLength = (mUsedSize * mOffset);
size_t byteLengthOfNewData = (mOffset * n);
size_t resizeArg = currentByteLength + (byteLengthOfNewData * 2);
_resize(resizeArg);

But _resize() begins with:
size_t newSize = size;
    char* newArr = new (std::nothrow) char[newSize * mOffset]; //multiplying by mOffset again
    //there's no check of (newArr == NULL)


_resize() does not destroy the contents of mFreeList before attempting to use the C&P code from the ctor to reset it.

//This shouldn't happen, since .....
Then it's an error condition and should be handled as such.


Is this subtraction correct?
inline const AllocTypePtr getPtr(unsigned i) const{
  return reinterpret_cast<AllocTypePtr>(reinterpret_cast<char*>(mAllocList.mFirst)
    - (mOffset * i) + sizeof(MemChunk));}


(OMG this forum bug... TO THE MOON, ALICE!)

#23Khatharr

Posted 31 December 2012 - 11:13 AM

allocate() calls _resize() in a way that basically boils down to:
size_t currentByteLength = (mUsedSize * mOffset);
size_t byteLengthOfNewData = (mOffset * n);
size_t resizeArg = currentByteLength + (byteLengthOfNewData * 2);
_resize(resizeArg);

But _resize() begins with:
size_t newSize = size;
    char* newArr = new (std::nothrow) char[newSize * mOffset]; //multiplying by mOffset again
    //there's no check of (newArr == NULL)


_resize() does not destroy the contents of mFreeList before attempting to use the C&P code from the ctor to reset it.


//This shouldn't happen, since .....
Then it's an error condition and should be handled as such.


Is this subtraction correct?

inline const AllocTypePtr getPtr(unsigned i) const{
  return reinterpret_cast<AllocTypePtr>(reinterpret_cast<char*>(mAllocList.mFirst)
    - (mOffset * i) + sizeof(MemChunk));}


Overall, I think you'll have a much easier time if you refactor this to avoid a couple issues:

  • Member variable names are confusing. I renamed many of them while I was reading through this in VS so that I could understand what was going on. For instace, mOffset is not an offset. It's the size of an entry.[/*]
  • You're using the MemChunk struct to make your list and storing the data between nodes. Why not just add a 'data' member to MemChunk so that you don't have to use error-prone pointer arithmetic to get at the node's payload?[/*]
  • When using complex compound expressions it's best to break them into several small statements since this breaks the logic into bite-sized steps. It compiles the same, but you can read it much more easily and that makes logic errors jump out at you.[/*]

(OMG this forum bug... TO THE MOON, ALICE!)

#22Khatharr

Posted 31 December 2012 - 11:12 AM

allocate() calls _resize() in a way that basically boils down to:<pre class="_prettyXprint _lang-auto _linenums:1">size_t currentByteLength = (mUsedSize * mOffset);
size_t byteLengthOfNewData = (mOffset * n);
size_t resizeArg = currentByteLength + (byteLengthOfNewData * 2);
_resize(resizeArg);</pre><br />But _resize() begins with:<pre class="_prettyXprint _lang-auto _linenums:1">size_t newSize = size;
char* newArr = new (std::nothrow) char[newSize * mOffset]; //multiplying by mOffset again
//there's no check of (newArr == NULL)</pre>&nbsp;<br />&nbsp;<br />_resize() does not destroy the contents of mFreeList before attempting to use the C&amp;P code from the ctor to reset it.<br />&nbsp;&nbsp;<br />&nbsp;<pre class="_prettyXprint _lang-auto _linenums:1">//This shouldn't happen, since .....</pre>Then it's an error condition and should be handled as such.<br /><br /><br />Is this subtraction correct?<br />&nbsp;&nbsp;<pre class="_prettyXprint _lang-auto _linenums:1">inline const AllocTypePtr getPtr(unsigned i) const{
return reinterpret_cast&lt;AllocTypePtr&gt;(reinterpret_cast&lt;char*&gt;(mAllocList.mFirst)
- (mOffset * i) + sizeof(MemChunk));}</pre>&nbsp;<br /><br />Overall, I think you'll have a much easier time if you refactor this to avoid a couple issues:<br />&nbsp;<ul class="bbc"><li>Member variable names are confusing. I renamed many of them while I was reading through this in VS so that I could understand what was going on. For instace, mOffset is not an offset. It's the size of an entry.</li><li>You're using the MemChunk struct to make your list and storing the data between nodes. Why not just add a 'data' member to MemChunk so that you don't have to use error-prone pointer arithmetic to get at the node's payload?</li><li>When using complex compound expressions it's best to break them into several small statements since this breaks the logic into bite-sized steps. It compiles the same, but you can read it much more easily and that makes logic errors jump out at you.</li></ul>(OMG this forum bug... TO THE MOON, ALICE!)

#21Khatharr

Posted 31 December 2012 - 11:06 AM

allocate() calls _resize() in a way that basically boils down to:
size_t newLength = mUsedSize + n;
    if(newLength > mMaxSize) {
        _resize((newLength * mOffset) * 2);
    }

But _resize() begins with:
size_t newSize = size;
    char* newArr = new (std::nothrow) char[newSize * mOffset]; //multiplying by mOffset again
    //there's no check of (newArr == NULL)
 
 

_resize() does not destroy the contents of mFreeList before attempting to use the C&P code from the ctor to reset it.
 
 
 
 
 
//This shouldn't happen, since .....
Then it's an error condition and should be handled as such.


Is this subtraction correct?
 
 
 
inline const AllocTypePtr getPtr(unsigned i) const{
  return reinterpret_cast<AllocTypePtr>(reinterpret_cast<char*>(mAllocList.mFirst)
    - (mOffset * i) + sizeof(MemChunk));}
 

Overall, I think you'll have a much easier time if you refactor this to avoid a couple issues:
 
  • Member variable names are confusing. I renamed many of them while I was reading through this in VS so that I could understand what was going on. For instace, mOffset is not an offset. It's the size of an entry.
  • You're using the MemChunk struct to make your list and storing the data between nodes. Why not just add a 'data' member to MemChunk so that you don't have to use error-prone pointer arithmetic to get at the node's payload?
  • When using complex compound expressions it's best to break them into several small statements since this breaks the logic into bite-sized steps. It compiles the same, but you can read it much more easily and that makes logic errors jump out at you.

(OMG this forum bug... TO THE MOON, ALICE!)

PARTNERS