I'm using dcmtk library to modify the pixel data of a multi frame compressed dicom image. So, to do that, at one stage in an for
loop I take the pixel data of each decompressed frame and modify them according my wish and try to concatenate each modify pixel data in a big memory buffer frame by frame. This core process of for
loop is as below.
The problem is after the first iteration it gives memory at the line of the code where I call the function getUncompressedFrame
. I think it's happening because of the line memcpy(fullBuffer+(i*sizeF),newBuffer,sizeF);
, as when I remove that line there's no error at that time and the whole for loop works absolutely fine.
Could you please say me if I'm making a mistake in working with memcpy? Thanks.
Uint32 sizeF=828072;// I just wrote it to show what is the data type.
Uint8 * fullBuffer = new Uint8(int(sizeF*numOfFrames));//The big memory buffer
for(int i=0;i<numOfFrames;i++)
{
Uint8 * buffer = new Uint8[int(sizeF)];//Buffer for each frame
Uint8 * newBuffer = new Uint8[int(sizeF)];//Buffer in which the modified frame data is stored
DcmFileCache * cache=NULL;
OFCondition cond=element->getUncompressedFrame(dataset,i,startFragment,buffer,sizeF,decompressedColorModel,cache);
//I get the uncompressed individual frame pixel data
if(buffer != NULL)
{
for(unsigned long y = 0; y < rows; y++)
{
for(unsigned long x = 0; x < cols; x++)
{
if(planarConfiguration==0)
{
if(x>xmin && x<xmax && y>ymin && y<ymax)
{
index=(x + y + y*(cols-1))*samplePerPixel;
if(index<sizeF-2)
{
newBuffer[index] = 0;
newBuffer[index + 1] = 0;
newBuffer[index +2] = 0;
}
}
else
{
index=(x + y + y*(cols-1))*samplePerPixel;
if(index<sizeF-2)
{
newBuffer[index] = buffer[index];
newBuffer[index + 1] = buffer[index + 1];
newBuffer[index + 2] = buffer[index + 2];
}
}
}
}
}
memcpy(fullBuffer+(i*sizeF),newBuffer,sizeF);
//concatenate the modified frame by frame pixel data
}
Because so many buffer overruns, and thus potential security exploits, have been traced to improper usage of memcpy , this function is listed among the "banned" functions by the Security Development Lifecycle (SDL).
If you give it valid pointers that do not overlap and you do not overrun the buffers with your reads/writes, it will not fail. If you do some of those things, it may still not fail, but could do unexpected things. It will always return dest.
memmove() is similar to memcpy() as it also copies data from a source to destination.
The memcpy() function shall return s1; no return value is reserved to indicate an error.
Change the declaration of fullBuffer
to this:
Uint8 * fullBuffer = new Uint8[int(sizeF*numOfFrames)];
Your code didn't allocate an array, it allocated a single Uint8
with the value int(sizeF*numOfFrames)
.
Uint8 * fullBuffer = new Uint8(int(sizeF*numOfFrames));
This allocates a single byte, giving it an initial value of sizeF*numOfFrames
(after truncating it first to int
and then to Uint8
). You want an array, and you don't want to truncate the size to int
:
Uint8 * fullBuffer = new Uint8[sizeF*numOfFrames];
^ ^
or, to fix the likely memory leaks in your code:
std::vector<Uint8> fullBuffer(sizeF*numOfFrames);
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With