memory leaks
Larry
do you think the following code could lead to memory leaks? it creates a
circualr buffer made up of 5 element then it pushes 10 elements (overwriting
the first 5) when overwriting , do you think the eldest pointed memory will
be overwritten, or a new pointer in memory will be written?
#define _CRT_SECURE_NO_WARNINGS
#include <windows.h>
#include <vector>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;
char * getDateTime(void);
const short numbuff = 5;
const short buflen = 30;
typedef struct
{
unsigned char * pData;
unsigned short bufferLength;
unsigned short bytesRecorded;
bool flag;
} Buffer;
int main()
{
circular_buffer<Buffer*> cb(numbuff);
circular_buffer<Buffer*>::const_iterator it;
printf("Push elements:\n");
// fill buffer
for(int i = 0; i<10; i++)
{
// set up buffer
Buffer *buff = new Buffer;
ZeroMemory(buff, sizeof(Buffer));
buff->bufferLength = buflen;
buff->bytesRecorded = buflen;
buff->flag = true;
buff->pData = new unsigned char[buflen];
buff->pData = reinterpret_cast<unsigned char *>(getDateTime());
printf("%s\n", buff->pData);
// push buffer
cb.push_back(buff);
Sleep(1000);
}
printf("\nShow elements:\n");
// show elements
for(int i = 0; i<static_cast<int>(cb.size()); i++)
{
printf("%s\n", cb[i]->pData);
}
system("pause");
return EXIT_SUCCESS;
}
// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
char * getDateTime(void)
{
time_t rawtime;
struct tm * timeinfo;
time(&rawtime);
timeinfo = gmtime(&rawtime);
char * buffer = new char[30];
strftime(buffer,30,"%a, %d %b %Y %X GMT",timeinfo);
return buffer;
}
// thanks
Alf P. Steinbach
> Hi,
>
> do you think the following code could lead to memory leaks? it creates
> a circualr buffer made up of 5 element then it pushes 10 elements
> (overwriting the first 5) when overwriting , do you think the eldest
> pointed memory will be overwritten, or a new pointer in memory will be
> written?
The question is unclear, but yes, you have memory leaks.
The memory leaks are due to coding at the C level.
In C++ use library classes, such as replacing your Buffer with std::string. :-)
> #define _CRT_SECURE_NO_WARNINGS
Don't do that.
> #include <windows.h>
> #include <vector>
> #include <cstdlib>
> #include <ctime>
> #include <cstdio>
> #include <boost/circular_buffer.hpp>
> using namespace std;
> using namespace boost;
>
> char * getDateTime(void);
Let that function return std::string.
By the way, 'void' as argument type is a C-ism.
It doesn't make much sense in C++.
> const short numbuff = 5;
> const short buflen = 30;
This confuses two different kinds of buffers: your circular buffer with room for
5 strings, and your string type named "Buffer" with room for 30 characters.
You don't need the latter constant.
For the first constant, consider replacing 'short' with 'int': there's no good
reason to choose anything but 'int' here.
> typedef struct
> {
> unsigned char * pData;
> unsigned short bufferLength;
> unsigned short bytesRecorded;
> bool flag;
> } Buffer;
Just remove that.
> int main()
> {
> circular_buffer<Buffer*> cb(numbuff);
This would be
circular_buffer<string> timeStrings( bufferSize );
> circular_buffer<Buffer*>::const_iterator it;
>
> printf("Push elements:\n");
> // fill buffer
> for(int i = 0; i<10; i++)
> {
> // set up buffer
> Buffer *buff = new Buffer;
> ZeroMemory(buff, sizeof(Buffer));
>
> buff->bufferLength = buflen;
> buff->bytesRecorded = buflen;
> buff->flag = true;
> buff->pData = new unsigned char[buflen];
> buff->pData = reinterpret_cast<unsigned char *>(getDateTime());
Don't cast.
>
> printf("%s\n", buff->pData);
>
> // push buffer
> cb.push_back(buff);
This would be simply
timeStrings.push_back( getDateTime() );
> Sleep(1000);
> }
>
> printf("\nShow elements:\n");
>
> // show elements
> for(int i = 0; i<static_cast<int>(cb.size()); i++)
> {
> printf("%s\n", cb[i]->pData);
> }
>
> system("pause");
> return EXIT_SUCCESS;
> }
>
> // getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
> char * getDateTime(void)
> {
> time_t rawtime;
> struct tm * timeinfo;
> time(&rawtime);
> timeinfo = gmtime(&rawtime);
> char * buffer = new char[30];
> strftime(buffer,30,"%a, %d %b %Y %X GMT",timeinfo);
> return buffer;
> }
See above.
Cheers & hth.,
- Alf
SG
> Hi,
> do you think the following code could lead to memory leaks?
Yes. In this case it's fairly easy to determine: You used the "new"-
operator three times but never the "delete"-operator and never passed
on the responsibility for managing the object life times to other
functions or objects.
> typedef struct
> {
> unsigned char * pData;
> unsigned short bufferLength;
> unsigned short bytesRecorded;
> bool flag;
> } Buffer;
Note: The type "Buffer" isn't responsible for managing the allocated
buffer. Otherwise you would have made the members private and added
allocation/deallocation/copy ctor/assignment operators.
> int main()
> {
> circular_buffer<Buffer*> cb(numbuff);
> circular_buffer<Buffer*>::const_iterator it;
>
> printf("Push elements:\n");
> // fill buffer
> for(int i = 0; i<10; i++)
> {
> // set up buffer
> Buffer *buff = new Buffer;
These Buffer objects are never deleted.
> ZeroMemory(buff, sizeof(Buffer));
>
> buff->bufferLength = buflen;
> buff->bytesRecorded = buflen;
> buff->flag = true;
> buff->pData = new unsigned char[buflen];
These chunks of memory buff->pData points to are never deleted.
> // push buffer
> cb.push_back(buff);
The comment is wrong. You don't push a buffer. You push a pointer to a
struct that contains a pointer to the "buffer". That's the problem.
That's your leak. The container simply stores these pointers and is
not responsible for deleting your stuff. Containers generally don't
care about what kind of objects they store and they don't treat
pointers specially. Note: "object" and "pointer" are not mutually
exclusive. You can have an object that IS a pointer and containers a
la vector<int*> stores pointer objects. If you destroy a pointer,
nothing special happens -- specifically, the pointed to object is not
touched or free'd automatically.
> // getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
> char * getDateTime(void)
> {
> time_t rawtime;
> struct tm * timeinfo;
> time(&rawtime);
> timeinfo = gmtime(&rawtime);
> char * buffer = new char[30];
> strftime(buffer,30,"%a, %d %b %Y %X GMT",timeinfo);
> return buffer;
> }
Yet another new[] for which I don't see a delete[].
The rule is simple: If you new'd something you should delete it again
-- expect when you pass the pointer to some function or object that
_specifically_ takes ownership (responsibility for managing the life-
time) of the POINTEE (the thing the pointer points to).
Cheers,
SG
Larry
news:hjkg1c$88b$1...@news.eternal-september.org...
> The question is unclear, but yes, you have memory leaks.
>
> The memory leaks are due to coding at the C level.
>
> In C++ use library classes, such as replacing your Buffer with
> std::string. :-)
That's great of you! Yet, I might be dealing with some data returned to me
as <unsigned char>. If it is possible to cast from unsigned char to string I
will be totally follow your advice. (I am dwaling with binary data mostly!)
thanks
Alf P. Steinbach
In practice you'll have to cast the pointer then, because
std::basic_string<unsigned char> is somewhere in the gray zone of not quite well
specified.
But other than that, the std::string constructors let you copy the data as
zero-terminated or as n bytes, whatever you have.
And std::string deals well with zero bytes, no problems.
However, if this is just raw data, consider
typedef unsigned char Byte;
typedef vector<Byte> ByteVector;
Bo Persson
So, if it is not a string you could try std::vector<unsigned char>
instead. That's a good type for a byte buffer.
Bo Persson
Larry
"Bo Persson" <b...@gmb.dk> ha scritto nel messaggio
news:7s60mg...@mid.individual.net...
> So, if it is not a string you could try std::vector<unsigned char>
> instead. That's a good type for a byte buffer.
That's what I have been looking for!
Anyway now I have trouble with the following:
void getDateTime(char * szTime);
const int numbuff = 5;
const int buflen = 30;
struct Buffer
{
public:
vector<char> vChar;
unsigned int bufferLength;
unsigned int bytesRecorded;
Buffer() : bytesRecorded(0), bufferLength(0), vChar(NULL) { };
};
int main()
{
circular_buffer<Buffer> cb(numbuff);
circular_buffer<Buffer>::const_iterator it;
for(int i = 0; i<10; i++)
{
// Get time
char szTime[30]; getDateTime(szTime);
// Init Buff
Buffer buff;
ZeroMemory(&buff, sizeof(Buffer));
buff.vChar.resize(buflen);
buff.vChar = szTime;
buff.bufferLength = buflen;
buff.bytesRecorded = buflen;
printf("%s\n", buff.vChar);
}
system("pause");
return EXIT_SUCCESS;
}
// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
void getDateTime(char * szTime)
{
time_t rawtime = time(NULL);
struct tm timeinfo;
gmtime_s(&timeinfo, &rawtime);
strftime(szTime, 30, "%a, %d %b %Y %X GMT", &timeinfo);
}
The code fails here: buff.vChar = szTime;
???
thanks
Larry
> However, if this is just raw data, consider
>
> typedef unsigned char Byte;
> typedef vector<Byte> ByteVector;
the new struct should look like this:
struct Buffer
{
public:
vector<unsigned char> vuChar;
int bufferLength;
int bytesRecorded;
int user;
Buffer() : bytesRecorded(0), bufferLength(0), vChar(NULL), user(0) { };
};
what the difference between:
--> vector<unsigned char> vuChar;
and
--> unsigne char buffer[]
??
thanks
Richard Herring
<dontme...@got.it> writes
[aside: why is everyone posting low-level C code this week?]
>
>> So, if it is not a string you could try std::vector<unsigned char>
>>instead. That's a good type for a byte buffer.
>
>That's what I have been looking for!
You're using strftime and printf("%s") on it. That looks like a string
to me.
>
>Anyway now I have trouble with the following:
>
>void getDateTime(char * szTime);
Can't you make this into a function that returns a std::string?
>const int numbuff = 5;
>const int buflen = 30;
>
>struct Buffer
>{
>public:
>vector<char> vChar;
>unsigned int bufferLength;
>unsigned int bytesRecorded;
What's the point of bufferLength and bytesRecorded now? vector does its
own housekeeping, so there's no need for you to.
>Buffer() : bytesRecorded(0), bufferLength(0), vChar(NULL) { };
Lose that. vector's default constructor will work fine.
>};
>
>int main()
>{
>circular_buffer<Buffer> cb(numbuff);
>circular_buffer<Buffer>::const_iterator it;
You declare these but never use them.
>
>for(int i = 0; i<10; i++)
>{
And you do this 10 times for no good reason. I think you've lost track
of the need for a circular buffer somewhere along the way ;-(
> // Get time
> char szTime[30];
Is that 30 the same as the "buflen" you declared earlier?
Why the Hungarian prefix?
>getDateTime(szTime);
>
> // Init Buff
> Buffer buff;
> ZeroMemory(&buff, sizeof(Buffer));
Don't do that. Buffer's constructor should do all you need to do to
initialise it.
>
> buff.vChar.resize(buflen);
Don't do that. Assigning to the buffer will do all that's necessary.
> buff.vChar = szTime;
buff.vChar.assign(szTime, szTime+strlen(szTime));
Better still, give Buffer a constructor with appropriate arguments which
initialises the buffer.
> buff.bufferLength = buflen;
> buff.bytesRecorded = buflen;
Redundant. Just use buff.vChar.size();
>
> printf("%s\n", buff.vChar);
printf("%s\n", &buff.vChar[0]);
>}
>
>system("pause");
>return EXIT_SUCCESS;
>}
>
>// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
>void getDateTime(char * szTime)
>{
>time_t rawtime = time(NULL);
>struct tm timeinfo;
>gmtime_s(&timeinfo, &rawtime);
>strftime(szTime, 30, "%a, %d %b %Y %X GMT", &timeinfo);
Is that "30" the same as the "buflen" you declared earlier?
>}
>
>The code fails here: buff.vChar = szTime;
If you'd used string instead of vector<char> it would have worked ;-)
Because it's such a common operation, string (unlike vector) has an
assignment operator that takes a pointer to a C-style null-terminated
string.
>
>???
Try "Accelerated C++".
>
>thanks
>
--
Richard Herring
Larry
news:Vvdt2izr...@baesystems.com...
> And you do this 10 times for no good reason. I think you've lost track of
> the need for a circular buffer somewhere along the way ;-(
I am just tring the circular buffer overwritten.
anyway the following code does not work:
#include <windows.h>
#include <vector>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;
void getDateTime(char * szTime);
const int numbuff = 5;
const int buflen = 30;
struct Buffer
{
public:
vector<char> vChar;
int bufferLength;
int bytesRecorded;
int user;
Buffer() : bytesRecorded(0), bufferLength(0), user(0) { };
};
int main()
{
circular_buffer<Buffer> cb(numbuff);
circular_buffer<Buffer>::const_iterator it;
// Insert elements
for(int i = 0; i<10; i++)
{
// Get time
char szTime[30]; getDateTime(szTime);
// Init Buff
Buffer buff;
copy(&szTime[0],&szTime[30],std::back_inserter(buff.vChar));
//buff.vChar.assign(szTime, szTime+strlen(szTime));
buff.bufferLength = buflen;
buff.bytesRecorded = buflen;
buff.user = i;
printf("%d, %d, %s\n", buff.user, buff.bufferLength, szTime);
cb.push_back(buff);
}
// Show elements:
for(int i = 0; i<(int)cb.size(); i++)
{
printf("%d, %d, %s\n", cb[i].user, cb[i].bufferLength, cb[i].vChar);
}
system("pause");
return EXIT_SUCCESS;
}
// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
void getDateTime(char * szTime)
{
time_t rawtime = time(NULL);
struct tm timeinfo;
gmtime_s(&timeinfo, &rawtime);
strftime(szTime, 30, "%a, %d %b %Y %X GMT", &timeinfo);
}
in the second loop I get <null> when I try to print vChar
NB: in the near future I may be deal with unsigned char data so that's why I
cannot have getDateTime() return std::string
Larry
news:cd64ad64-151b-44a3...@p24g2000yqm.googlegroups.com...
> Yes. In this case it's fairly easy to determine: You used the "new"-
> operator three times but never the "delete"-operator and never passed
> on the responsibility for managing the object life times to other
> functions or objects.
Ok! do you think the following may still lead to memory leaks?
#include <windows.h>
#include <vector>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;
void getDateTime(char * szTime);
const int numbuff = 5;
const int buflen = 30;
struct Buffer
{
public:
char * payload;
int bufferLength;
int bytesRecorded;
int user;
Buffer() : bytesRecorded(0), bufferLength(0), user(0), payload(NULL) { };
};
int main()
{
circular_buffer<Buffer> cb(numbuff);
// Insert elements
for(int i = 0; i<10; i++)
{
// Get time
char szTime[30]; getDateTime(szTime);
// Init Buff
Buffer buff;
buff.user = i;
buff.payload = szTime;
buff.bufferLength = buflen;
buff.bytesRecorded = buflen;
cb.push_back(buff);
}
// Show elements:
for(int i = 0; i<(int)cb.size(); i++)
{
printf("%d, %d, %s\n", cb[i].user, cb[i].bufferLength, cb[i].payload);
}
system("pause");
return EXIT_SUCCESS;
}
// getDateTime (Fri, 10 Oct 2008 14:41:59 GMT)
void getDateTime(char * szTime)
{
time_t rawtime = time(NULL);
struct tm timeinfo;
gmtime_s(&timeinfo, &rawtime);
strftime(szTime, 30, "%a, %d %b %Y %X GMT", &timeinfo);
}
thanks
Thomas J. Gritzan
> struct Buffer
> {
> public:
> vector<char> vChar;
> int bufferLength;
> int bytesRecorded;
> int user;
> Buffer() : bytesRecorded(0), bufferLength(0), user(0) { };
> };
>
> int main()
> {
> circular_buffer<Buffer> cb(numbuff);
> circular_buffer<Buffer>::const_iterator it;
>
> // Insert elements
> for(int i = 0; i<10; i++)
> {
> // Get time
> char szTime[30]; getDateTime(szTime);
Use your constants:
char szTime[buflen];
> // Init Buff
> Buffer buff;
> copy(&szTime[0],&szTime[30],std::back_inserter(buff.vChar));
This also works:
buff.vChar.assign(szTime, szTime+buflen);
> //buff.vChar.assign(szTime, szTime+strlen(szTime));
> buff.bufferLength = buflen;
> buff.bytesRecorded = buflen;
> buff.user = i;
>
> printf("%d, %d, %s\n", buff.user, buff.bufferLength, szTime);
>
> cb.push_back(buff);
> }
>
> // Show elements:
> for(int i = 0; i<(int)cb.size(); i++)
> {
> printf("%d, %d, %s\n", cb[i].user, cb[i].bufferLength, cb[i].vChar);
> }
[...]
> in the second loop I get <null> when I try to print vChar
Of course. You try to output a std::vector through "%s", but a vector
isn't a C-style string. Learn to use C++ facilities:
std::cout << cb[i].user << ", " << cb[i].bufferLength << ", " <<
cb[i].vChar << std::endl;
This doesn't compile either, which is good, because is gives you an
error message, while the same with printf() just silently compiles and
yields undefined behaviour. So compiler-errors are good,
this-just-doesnt-work-and-I-dont-know-why is very bad.
Since you are storing a C-style string in the vector, you can do this to
output the string (works also with printf):
std::cout << &cb[i].vChar[0];
--
Thomas
Alf P. Steinbach
> "Alf P. Steinbach" <al...@start.no> ha scritto nel messaggio
> news:hjki9g$rc0$1...@news.eternal-september.org...
>> * Larry:
>>> "Alf P. Steinbach" <al...@start.no> ha scritto nel messaggio
>
>> However, if this is just raw data, consider
>>
>> typedef unsigned char Byte;
>> typedef vector<Byte> ByteVector;
>
> the new struct should look like this:
>
> struct Buffer
> {
> public:
> vector<unsigned char> vuChar;
> int bufferLength;
> int bytesRecorded;
> int user;
> Buffer() : bytesRecorded(0), bufferLength(0), vChar(NULL), user(0) { };
> };
More like
typedef unsigned char Byte;
struct Something
{
vector<Byte> bytes;
int userId;
// Constructor etc.
};
> what the difference between:
> --> vector<unsigned char> vuChar;
> and
> --> unsigne char buffer[]
The declaration
unsigned char buffer[51];
says that 'buffer' is a contiguous area of memory consisting of 51 elements each
of type 'unsigned char'.
The declaration
vector<unsigned char> v( 51 );
says that 'v' is a 'vector' which internally holds a pointer to a buffer, and
that on construction it should create a buffer of 51 elements. This can be
resized (if necessary the vector will discard the original buffer and copy
everything over to a and sufficiently larger buffer). And you can assign to 'v'
and generally copy 'v', and be sure that there is no memory leak.
Larry
news:hjkru0$o2i$1...@newsreader3.netcologne.de...
>> struct Buffer
>> {
>> public:
>> vector<char> vChar;
>> int bufferLength;
>> int bytesRecorded;
>> int user;
>> Buffer() : bytesRecorded(0), bufferLength(0), user(0) { };
>> };
So I think the following is the only way not to have memory leaks:
#include <windows.h>
#include <cstdlib>
#include <ctime>
#include <cstdio>
#include <cstring>
#include <boost/circular_buffer.hpp>
using namespace std;
using namespace boost;
void getDateTime(char * szTime);
const int numbuff = 3;
const int buflen = 30;
struct Buffer
{
public:
char payload[4096];
int bytesRecorded;
int user;
Buffer() : bytesRecorded(0), user(0) { }
};
int main()
{
circular_buffer<Buffer> cb(numbuff);
// Insert elements
printf("Push elements:\n");
for(int i = 0; i<5; i++)
{
// Get time
char szTime[30]; getDateTime(szTime);
// Init Buff
Buffer buff;
ZeroMemory(&buff, sizeof(Buffer));
memcpy(static_cast<void*>(buff.payload), static_cast<void*>(szTime),
buflen);
buff.user = i;
buff.bytesRecorded = buflen;
cb.push_back(buff);
printf("%s\n", buff.payload);
Sleep(1000);
}
// Show elements:
printf("Show elements:\n");
for(int i = 0; i<(int)cb.size(); i++)
{
printf("%s\n", cb[i].payload);
}
system("pause");
return EXIT_SUCCESS;
}
void getDateTime(char * szTime)
{
time_t rawtime = time(NULL);
struct tm timeinfo;
gmtime_s(&timeinfo, &rawtime);
strftime(szTime, 30, "%a, %d %b %Y %X GMT", &timeinfo);
}
where payload is set to 4096 bytes long. Indeed it's going to hold less data
but this way I can make sure it can hold from 0 to 4095 bytes...the other
filed in the struct will store how long payload actually is!
thanks
Rolf Magnus
>> Yes. In this case it's fairly easy to determine: You used the "new"-
>> operator three times but never the "delete"-operator and never passed
>> on the responsibility for managing the object life times to other
>> functions or objects.
>
> Ok! do you think the following may still lead to memory leaks?
Since that code doesn't allocate memory anymore, it can't leak memory. But
you are making other severe memory access errors.
Note that szTime is local to the loop. It stops existing once the current
loop iteration is done.
> buff.bufferLength = buflen;
> buff.bytesRecorded = buflen;
>
> cb.push_back(buff);
> }
>
> // Show elements:
> for(int i = 0; i<(int)cb.size(); i++)
> {
> printf("%d, %d, %s\n", cb[i].user, cb[i].bufferLength, cb[i].payload);
Here you're accessing objects that don't exist anymore. Anything can happen
here. Why are you using raw arrays of char instead of std::string?
Memory allocation is an advanced topic, so avoid doing it yourself and let
the standard classes handle it for you. That will make it a lot easier.
Nick Keighley
> * Larry:
> > #define _CRT_SECURE_NO_WARNINGS
>
> Don't do that.
why?
It suppresses some irritating MS warnings. Are you saying don't
suppress the warnings or don't embed such MSisms in your source (for
instance use something in the build system like
-D?)
Richard Herring
<dontme...@got.it> writes
>"Thomas J. Gritzan" <phygon_...@gmx.de> ha scritto nel messaggio
>news:hjkru0$o2i$1...@newsreader3.netcologne.de...
>
>>> struct Buffer
>>> {
>>> public:
>>> vector<char> vChar;
>>> int bufferLength;
>>> int bytesRecorded;
>>> int user;
>>> Buffer() : bytesRecorded(0), bufferLength(0), user(0) { };
>>> };
>
>So I think the following is the only way not to have memory leaks:
[...]
>
>struct Buffer
>{
>public:
>char payload[4096];
[...]
>
>where payload is set to 4096 bytes long. Indeed it's going to hold less
>data but this way I can make sure it can hold from 0 to 4095
>bytes...the other filed in the struct will store how long payload
>actually is!
<sigh>
And when you change the program so it needs 4097 bytes, you will get
undefined behaviour. What is so hard about using vector or string to
manage that memory, as everyone else is advising you?
USE A VECTOR.
--
Richard Herring
James Kanze
> * Larry:
> > "Alf P. Steinbach" <al...@start.no> ha scritto nel messaggio
> >news:hjki9g$rc0$1...@news.eternal-september.org...
> More like
> typedef unsigned char Byte;
> struct Something
> {
> vector<Byte> bytes;
> int userId;
> // Constructor etc.
> };
> > what the difference between:
> > --> vector<unsigned char> vuChar;
> > and
> > --> unsigne char buffer[]
> The declaration
> unsigned char buffer[51];
> says that 'buffer' is a contiguous area of memory consisting
> of 51 elements each of type 'unsigned char'.
> The declaration
> vector<unsigned char> v( 51 );
> says that 'v' is a 'vector' which internally holds a pointer
> to a buffer, and that on construction it should create a
> buffer of 51 elements. This can be resized (if necessary the
> vector will discard the original buffer and copy everything
> over to a and sufficiently larger buffer). And you can assign
> to 'v' and generally copy 'v', and be sure that there is no
> memory leak.
In fact, "vector< unsigned char >" is full-fledged type, which
behaves like any other type, where as "unsigned char buffer[ 51 ]"
creates an object of a second class type, which is fairly
broken, and doesn't behave normally.
Also, vector< unsigned char > will normally do bounds checking,
and other important things to avoid undefined behavior.
There are, of course, cases where something like "unsigned char
buffer[ 51 ]" is justified, but they aren't that common.
--
James Kanze
Alf P. Steinbach
I probably agree with what you mean, but the wording above might give the wrong
impression to the OP or other readers.
The type of "unsigned char buffer[52]" is technically a full-fledged type. It's
just that, as you note, this type is broken, due to C compatibility. So with
respect to what one can do and with respect to type safety it's not quite up to par.
In particular, regarding technical type full fledgeness :-) (not you don't know
this, but to avoid any misunderstanding), given
unsigned char buffer[51];
one might apply typeid(), or let some templated function infer the type,
whatever -- with respect to the C++ type system there are no not full fledged
types except incomplete types (types where sizeof cannot be applied) and perhaps
to some degree local classes because they don't have linkage and so do not go
well with templating.
> Also, vector< unsigned char > will normally do bounds checking,
> and other important things to avoid undefined behavior.
>
> There are, of course, cases where something like "unsigned char
> buffer[ 51 ]" is justified, but they aren't that common.
Yes, agreed.
Cheers,
- Alf
io_x
"James Kanze" <james...@gmail.com> ha scritto nel messaggio
news:4d4a6dae-1a50-4e01...@f12g2000yqn.googlegroups.com...
> Also, vector< unsigned char > will normally do bounds checking,
> and other important things to avoid undefined behavior.
this seems untrue for std::vector<T> data_;
for T == struct Color3d{double r, g, b;};
#include <iostream>
#include <cstdlib>
#include <stdint.h>
#include <vector>
#define u8 uint8_t
#define i8 int8_t
#define u32 uint32_t
#define i32 int32_t
// float 32 bits
#define f32 float
#define S sizeof
#define R return
#define P printf
#define F for
using namespace std;
template <typename T>
class Image
{
class Indexer
{
public:
Indexer(T* data) : data_(data)
{
}
T& operator[](int x) const
{
return data_[x];
}
private:
T* data_;
};
class ConstIndexer
{
public:
ConstIndexer(const T* data) : data_(data)
{
}
T operator[](int x) const
{
return data_[x];
}
private:
const T* data_;
};
public:
Image(int width, int height) :
width_(width),
height_(height),
data_(width*height)
{
}
int width() const
{
return width_;
}
int height() const
{
return height_;
}
Indexer operator[](int y)
{
return Indexer(&data_[y*width_]);
}
ConstIndexer operator[](int y) const
{
return ConstIndexer(&data_[y*width_]);
}
private:
int width_;
int height_;
std::vector<T> data_;
};
struct Color3d
{
double r, g, b;
};
void fill(Image<Color3d>& img, Color3d color)
{
for (int y = 0; y < img.height(); ++y)
for (int x = 0; x < img.width(); ++x)
img[y][x] = color;
}
int main(void)
{Color3d h={0.78373, 0.1383, 1-0.78373-0.1383};
Image<Color3d> g(768, 1024);
// cout << "a=" << Image::a << "\n" ;
cout << "Inizio\n";
fill(g, h);
cout << "(r,g,b)==(" << g[0][0].r << ", "
<< g[0][0].g << ", "
<< g[768][1024].b << ")\n";
cout << "end\n";
return 0;
}
-------
result
Inizio
(r,g,b)==(0.78373, 0.1383, 0.07797)
end
Richard Herring
<a...@b.c.invalid> writes
>
>"James Kanze" <james...@gmail.com> ha scritto nel messaggio
>news:4d4a6dae-1a50-4e01...@f12g2000yqn.googlegroups.com...
>> Also, vector< unsigned char > will normally do bounds checking,
>> and other important things to avoid undefined behavior.
>
>this seems untrue for std::vector<T> data_;
>for T == struct Color3d{double r, g, b;};
>
>
>#include <iostream>
>#include <cstdlib>
>#include <stdint.h>
>#include <vector>
>
>#define u8 uint8_t
>#define i8 int8_t
>#define u32 uint32_t
>#define i32 int32_t
>// float 32 bits
>#define f32 float
>
>#define S sizeof
>#define R return
>#define P printf
>#define F for
Sorry, I stopped reading here. That kind of macro abuse merely makes the
rest of your code unreadable.
What point were you trying to make?
--
Richard Herring
Larry
news:4d4a6dae-1a50-4e01...@f12g2000yqn.googlegroups.com...
> In fact, "vector< unsigned char >" is full-fledged type, which
> behaves like any other type, where as "unsigned char buffer[ 51 ]"
> creates an object of a second class type, which is fairly
> broken, and doesn't behave normally.
>
> Also, vector< unsigned char > will normally do bounds checking,
> and other important things to avoid undefined behavior.
>
> There are, of course, cases where something like "unsigned char
> buffer[ 51 ]" is justified, but they aren't that common.
well, I could have done with vectors indeed. Yet, I thought becauese of all
those internal checks vector does maybe my solution would have been
faster...
where:
unsigned char buffer[51];
is fixed length. Also, I know that it will never deal with data longer that
51 bytes beforhand!
I am doing real time application I need to run fast.
thanks
Christian Hackl
> well, I could have done with vectors indeed. Yet, I thought becauese of all
> those internal checks vector does maybe my solution would have been
> faster...
You must not guess but measure.
It is unlikely that you will notice any difference in speed.
> unsigned char buffer[51];
>
> is fixed length. Also, I know that it will never deal with data longer that
> 51 bytes beforhand!
>
> I am doing real time application I need to run fast.
Even if std::vector should turn out to be too slow for you, which,
again, is unlikely, there are more elegant solutions, e.g. boost::array.
Also, you should never say "never" when it comes to software
requirements. Chances are that sooner or later you will have to deal
with data longer than 51 bytes.
--
Christian Hackl
ha...@sbox.tugraz.at
Milano 2008/2009 -- L'Italia chiam�, s�!
Jorgen Grahn
> Larry ha scritto:
>
>> well, I could have done with vectors indeed. Yet, I thought becauese of all
>> those internal checks vector does maybe my solution would have been
>> faster...
>
> You must not guess but measure.
>
> It is unlikely that you will notice any difference in speed.
Yes. And also, *which* "all those internal checks"? People usually
complain about the *lack* of checks in the standard containers, e.g.
range checks.
/Jorgen
--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .
Jorgen Grahn
...
> #define _CRT_SECURE_NO_WARNINGS
> #include <windows.h>
> #include <vector>
> #include <cstdlib>
> #include <ctime>
> #include <cstdio>
> #include <boost/circular_buffer.hpp>
> using namespace std;
> using namespace boost;
...
> ZeroMemory(buff, sizeof(Buffer));
Just out of curiosity: did you define this one somewhere, or are
Microsoft daft enough to push a differently named std::memset()?
(Or bzero(), although that one is just traditional).
More helpfully, in the rare case that I have to manually initialize
buffers like this, I use std::fill and friends. The old C functions
which operate on raw memory (memset, memcpy etc) doesn't play nice with
C++ objects, which in turn can lead to nasty bugs.
red floyd
> On Mon, 2010-01-25, Larry wrote:
> > ZeroMemory(buff, sizeof(Buffer));
>
> Just out of curiosity: did you define this one somewhere, or are
> Microsoft daft enough to push a differently named std::memset()?
Microsoft is that daft.
Alf P. Steinbach
I agree :-), but in this particular case it's not so. ZeroMemory is a Windows
API function. It's available whether you program in C or Pascal or Fortran, say
(actually I don't know about Fortran, it's near 30 years since I last rode that
beast, but I'm pretty sure it can be used also from Fortran).
The OP's code was
Buffer *buff = new Buffer;
ZeroMemory(buff, sizeof(Buffer));
and by the rules of C++ he could just have written
Buffer* buff = new Buffer(); // That's it.
by �5.3.4/15 second main dash, which in C++98 reads "If the new-initializer is
of the form (), default-initialization shall be performed" (and I guess that's
changed to "value initialization" in C++03, but I'm not checking that).
Cheers,
- Alf
Rolf Magnus
> On Mon, 2010-01-25, Larry wrote:
> ...
>> #define _CRT_SECURE_NO_WARNINGS
>> #include <windows.h>
>> #include <vector>
>> #include <cstdlib>
>> #include <ctime>
>> #include <cstdio>
>> #include <boost/circular_buffer.hpp>
>> using namespace std;
>> using namespace boost;
>
> ...
>> ZeroMemory(buff, sizeof(Buffer));
>
> Just out of curiosity: did you define this one somewhere, or are
> Microsoft daft enough to push a differently named std::memset()?
They have their own function for pretty much everything, just like they have
CHAR and VOID as replacement for char and void. Maximum incompatibility
seems to be the main goal.
Jorgen Grahn
OK. To be fair, that's what my code looked like when I programmed in C
on the Commodore-Amiga too; their interfaces were full of CHAR and
WORD, and the had plenty of homegrown functions like ZeroMemory()
above. But that was twenty years ago, and using the C standard library
was considered uncool and "too slow".