memory leaks

Discussion in 'C++' started by Larry, Jan 25, 2010.

  1. Larry

    Larry Guest

    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?

    #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->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
    Larry, Jan 25, 2010
    #1
    1. Advertising

  2. * Larry:
    > 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->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
    Alf P. Steinbach, Jan 25, 2010
    #2
    1. Advertising

  3. Larry

    SG Guest

    On 25 Jan., 17:01, "Larry" <> wrote:
    > 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
    SG, Jan 25, 2010
    #3
  4. Larry

    Larry Guest

    "Alf P. Steinbach" <> ha scritto nel messaggio
    news:hjkg1c$88b$-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
    Larry, Jan 25, 2010
    #4
  5. * Larry:
    > "Alf P. Steinbach" <> ha scritto nel messaggio
    > news:hjkg1c$88b$-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!)


    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;

    Cheers & hth.,

    - Alf
    Alf P. Steinbach, Jan 25, 2010
    #5
  6. Larry

    Bo Persson Guest

    Larry wrote:
    > "Alf P. Steinbach" <> ha scritto nel messaggio
    > news:hjkg1c$88b$-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


    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
    Bo Persson, Jan 25, 2010
    #6
  7. Larry

    Larry Guest

    "Bo Persson" <> ha scritto nel messaggio
    news:...

    > 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, Jan 25, 2010
    #7
  8. Larry

    Larry Guest

    "Alf P. Steinbach" <> ha scritto nel messaggio
    news:hjki9g$rc0$-september.org...
    >* Larry:
    >> "Alf P. Steinbach" <> 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) { };
    };

    what the difference between:
    --> vector<unsigned char> vuChar;
    and
    --> unsigne char buffer[]

    ??

    thanks
    Larry, Jan 25, 2010
    #8
  9. In message <4b5dcf0e$0$819$>, Larry
    <> writes
    >
    >"Bo Persson" <> ha scritto nel messaggio
    >news:...


    [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
    Richard Herring, Jan 25, 2010
    #9
  10. Larry

    Larry Guest

    "Richard Herring" <junk@[127.0.0.1]> ha scritto nel messaggio
    news:...

    > 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.user, cb.bufferLength, cb.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, Jan 25, 2010
    #10
  11. Larry

    Larry Guest

    "SG" <> ha scritto nel messaggio
    news:...

    > 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.user, cb.bufferLength, cb.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
    Larry, Jan 25, 2010
    #11
  12. Am 25.01.2010 19:42, schrieb Larry:
    > 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.user, cb.bufferLength, cb.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.user << ", " << cb.bufferLength << ", " <<
    cb.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.vChar[0];

    --
    Thomas
    Thomas J. Gritzan, Jan 25, 2010
    #12
  13. * Larry:
    > "Alf P. Steinbach" <> ha scritto nel messaggio
    > news:hjki9g$rc0$-september.org...
    >> * Larry:
    >>> "Alf P. Steinbach" <> 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.


    Cheers & hth.,

    - Alf
    Alf P. Steinbach, Jan 25, 2010
    #13
  14. Larry

    Larry Guest

    "Thomas J. Gritzan" <> ha scritto nel messaggio
    news:hjkru0$o2i$...

    >> 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.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
    Larry, Jan 25, 2010
    #14
  15. Larry

    Rolf Magnus Guest

    Larry wrote:

    >> 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.

    > #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;


    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.user, cb.bufferLength, cb.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.

    > }
    >
    > 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
    Rolf Magnus, Jan 26, 2010
    #15
  16. On 25 Jan, 16:14, "Alf P. Steinbach" <> wrote:
    > * 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?)
    Nick Keighley, Jan 26, 2010
    #16
  17. In message <4b5df6a2$0$1141$>, Larry
    <> writes
    >"Thomas J. Gritzan" <> ha scritto nel messaggio
    >news:hjkru0$o2i$...
    >
    >>> 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
    Richard Herring, Jan 26, 2010
    #17
  18. Larry

    James Kanze Guest

    On Jan 25, 7:42 pm, "Alf P. Steinbach" <> wrote:
    > * Larry:
    > > "Alf P. Steinbach" <> ha scritto nel messaggio
    > >news:hjki9g$rc0$-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
    James Kanze, Jan 27, 2010
    #18
  19. * James Kanze:
    > On Jan 25, 7:42 pm, "Alf P. Steinbach" <> wrote:
    >> * Larry:
    >>> "Alf P. Steinbach" <> ha scritto nel messaggio
    >>> news:hjki9g$rc0$-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.


    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
    Alf P. Steinbach, Jan 27, 2010
    #19
  20. In message <4b600a1d$0$828$>, io_x
    <> writes
    >
    >"James Kanze" <> ha scritto nel messaggio
    >news:...
    >> 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
    Richard Herring, Jan 27, 2010
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. ASP.Confused

    ASP.NET - Detecting memory leaks

    ASP.Confused, Jul 16, 2004, in forum: ASP .Net
    Replies:
    2
    Views:
    2,726
    Marina
    Jul 16, 2004
  2. =?Utf-8?B?RnJhbmsxMjEz?=

    Memory Leaks in ASP.NET

    =?Utf-8?B?RnJhbmsxMjEz?=, Mar 4, 2005, in forum: ASP .Net
    Replies:
    12
    Views:
    13,815
  3. James Hunter Ross

    Our memory leaks?

    James Hunter Ross, Oct 20, 2005, in forum: ASP .Net
    Replies:
    2
    Views:
    765
    Bruce Barker
    Oct 20, 2005
  4. Novice
    Replies:
    28
    Views:
    5,061
    Jon Skeet
    Jul 22, 2003
  5. Replies:
    4
    Views:
    107
Loading...

Share This Page