Thread: mysql_real_escape_string memory leak ?

  1. #1
    Registered User
    Join Date
    Nov 2006
    Posts
    55

    mysql_real_escape_string memory leak ?

    In this example:
    Code:
    char *_sql(const char *dataIn)
    {
       char *to = new char[(strlen(dataIn) * 2) + 1];
       mysql_real_escape_string(MySQL_Database_Connection__global, to, dataIn, strlen(dataIn));
       dataIn = '\0';
       return (to);
    }
    char szQueryText[MAX_SQL_LENGTH] = "";
    char *text = "Hello World";
    snprintf(szQueryText, sizeof(szQueryText), "UPDATE this_table SET this_field = '%s' WHERE this_value = 1 ;", _sql(text));
    My_result = MySQL__query(szQueryText);
    MySQL__endquery(My_result);
    Everything works great. Data is perfect. No errors, no issues. However... when I pull up windows task manager and look at how much memory the program that's running this code is using, every time this code is invoked, the memory goes up. Obviously, there's a memory leak somewhere. I'm not free'ing something right?

    If I change:
    Code:
    ", _sql(text));
    to
    Code:
    ", text);
    No memory going up at all, hence, in my opinion, no leak. So...

    Where in my char *_sql(const char *dataIn) function am I going wrong?

    Thanks.

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,412
    Perhaps the reason is that you failed to delete[] what you new[]ed.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Nov 2006
    Posts
    55
    Quote Originally Posted by laserlight View Post
    Perhaps the reason is that you failed to delete[] what you new[]ed.
    I cannot 'delete' after a return, correct? If I 'delete' before the return, nothing will get returned, right?

    I'm most likely not understanding. Is this what you're suggesting?
    Code:
    char *_sql(const char *dataIn)
    {
       char *to = new char[(strlen(dataIn) * 2) + 1];
       mysql_real_escape_string(MySQL_Database_Connection__global, to, dataIn, strlen(dataIn));
       dataIn = '\0';
       delete to;
       return (to);
    }

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,412
    No, that is wrong as you noticed, and also because delete[], not delete, should be used.

    One way is to save the result of the call to _sql in a variable, then after using that variable, use delete[] with it.

    However, a possibly better way:
    Code:
    std::string _sql(const char *dataIn)
    {
        std::string to(strlen(dataIn) * 2);
        mysql_real_escape_string(MySQL_Database_Connection__global, &to[0], dataIn, strlen(dataIn));
        return to;
    }
    then:
    Code:
    snprintf(szQueryText,
             sizeof(szQueryText),
             "UPDATE this_table SET this_field = '%s' WHERE this_value = 1 ;",
             _sql(text).c_str());
    Note that names that begin with an underscore followed by a lowercase letter is reserved to the implementation for use in the global and std namespaces, so _sql should be within your own namespace. Actually, if the underscore prefix is just to denote this as a helper function to avoid name conflicts, consider just renaming it (escape_string?) and defining it in an unnamed namespace.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    Nov 2006
    Posts
    55
    Compiler not liking "std::string to(strlen(dataIn) * 2);"

  6. #6
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,412
    Perhaps I have simply forgotten the interface of std::string. Fix it to have the string be of the desired length.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  7. #7
    Registered User
    Join Date
    Nov 2006
    Posts
    55
    Interesting...

    Okay, the following code works, and no more memory leaks:
    Code:
    std::string sql(const char *dataIn)
    {
       std::string a = dataIn;
       std::string b = dataIn;
       std::string c = " ";
       std::string to = a + b + c;
    
       if (to.length() < 1)
          to = "             ";
    
       mysql_real_escape_string(MySQL_Database_Connection__global, &to[0], dataIn, strlen(dataIn));
       return (to);
    }
    However, I'm not very keen on the whole "a + b + c" thing. I wish I knew of a way to set the 'string to' length.

    Any ideas?

    By the way, thank you laserlight for all your help. You've helped me out in the past and I was quite happy that you were the one that replied to this post.

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,412
    One way:
    Code:
    std::stringstream ss;
    ss << dataIn << dataIn << " ";
    std::string to = ss.str();
    By the way, to.length() < 1 is tantamount to to.empty(), and this is not possible because you have a space there.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,412
    Actually, looking at what you want to do, that's dumb. It should be as simple as:
    Code:
    std::string to(' ', strlen(dataIn) * 2);
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  10. #10
    Registered User
    Join Date
    Nov 2006
    Posts
    55
    Quote Originally Posted by laserlight View Post
    Actually, looking at what you want to do, that's dumb. It should be as simple as:
    Code:
    std::string to(' ', strlen(dataIn) * 2);
    ...and there it is:
    Code:
    std::string sql(const char *dataIn)
    {
       std::string to(" ", (strlen(dataIn) * 2) + 1);
       mysql_real_escape_string(MySQL_Database_Connection__global, &to[0], dataIn, strlen(dataIn));
       return (to);
    }

    Thank you. Works perfect.

  11. #11
    Registered User
    Join Date
    Aug 2010
    Location
    Poland
    Posts
    733
    Quote Originally Posted by xwielder View Post
    ...and there it is:
    Code:
    std::string sql(const char *dataIn)
    {
       std::string to(" ", (strlen(dataIn) * 2) + 1);
       mysql_real_escape_string(MySQL_Database_Connection__global, &to[0], dataIn, strlen(dataIn));
       return (to);
    }
    Thank you. Works perfect.
    No, it does not work perfectly. Why did you change it to double quotes? It calls a completely different constructor, taking first n characters of the given string.

  12. #12
    Registered User
    Join Date
    Nov 2006
    Posts
    55
    Quote Originally Posted by kmdv View Post
    No, it does not work perfectly.
    Well, it works perfectly for me. So far, no errors or anything funky happening.
    Quote Originally Posted by kmdv View Post
    Why did you change it to double quotes?
    Didn't think there was a difference, but as you say here:
    Quote Originally Posted by kmdv View Post
    It calls a completely different constructor, taking first n characters of the given string.
    I'm not understanding. Could you provide an example please? Only reason I ask is because using double quotes seems to working just fine.

    Thanks.

  13. #13
    Registered User
    Join Date
    Nov 2006
    Posts
    55
    Interesting... I was just doing some testing with double quotes and single quotes regarding your point.

    The program constantly crashes when I use single quotes. (Seems to break on malloc.c "C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\crt\src\malloc.c" on line 55, which is this:
    Code:
    __forceinline void * __cdecl _heap_alloc (size_t size)
    
    {
    
        if (_crtheap == 0) {
            _FF_MSGBANNER();    /* write run-time error banner */
            _NMSG_WRITE(_RT_CRT_NOTINIT);  /* write message */
            __crtExitProcess(255);  /* normally _exit(255) */
        }
    
        return HeapAlloc(_crtheap, 0, size ? size : 1);
    }
    Anyway.... using double quotes works with no crashes, and the program seems to be handling all data just fine.

    The only difference I see is that I use double quotes, digging down into the "to" string there is much more miscellaneous data attached at the end. However, since we're using "&to[0]", the last junk data isn't used.

    ::scratches head in bewilderment"

  14. #14
    Registered User
    Join Date
    Nov 2006
    Posts
    55
    Well, I just tried this:

    Code:
    std::string sql(const char *dataIn)
    {
       std::string returnVal = "";
       char *to = new char[(strlen(dataIn) * 2) + 1];
       mysql_real_escape_string(MySQL_Database_Connection__global, to, dataIn, strlen(dataIn));
       returnVal = to;
       delete [] to;
       return (returnVal);
    }
    And it seems to work quite well. Would you agree this be the best solution?

  15. #15
    Registered User
    Join Date
    Aug 2010
    Location
    Poland
    Posts
    733
    Quote Originally Posted by xwielder View Post
    Well, I just tried this:

    Code:
    std::string sql(const char *dataIn)
    {
       std::string returnVal = "";
       char *to = new char[(strlen(dataIn) * 2) + 1];
       mysql_real_escape_string(MySQL_Database_Connection__global, to, dataIn, strlen(dataIn));
       returnVal = to;
       delete [] to;
       return (returnVal);
    }
    And it seems to work quite well. Would you agree this be the best solution?
    Firstly, I would consider changing parameter type 'const char*' to 'const std::string&' (depending how often you use std::string and how often you call this function at all).

    Secondly, you use global variable here, so definately it is not a good solution to go with.

    Finally, there is a memory leak. mysql_real_escape_string probably does not throw, but the assignment:
    Code:
    returnVal = to;
    may throw std::bad_alloc() and delete[] will not be executed.

    Consider:
    - adding try-catch block
    - using std::vector<char> for 'to' buffer
    - using std::string for 'to' buffer. std::string is going to be guaranteed to have contiguous memory in the c++0x and at the moment most implementations implement it this way.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Memory Leak?
    By lovercjs in forum C Programming
    Replies: 5
    Last Post: 03-04-2011, 11:06 AM
  2. Memory Leak
    By jtullo in forum C Programming
    Replies: 7
    Last Post: 12-11-2006, 11:45 PM
  3. Replies: 2
    Last Post: 09-28-2006, 01:06 PM
  4. A memory leak (I think...)
    By Stevek in forum C++ Programming
    Replies: 7
    Last Post: 03-16-2003, 03:09 PM
  5. memory leak
    By stormbringer in forum C Programming
    Replies: 7
    Last Post: 07-17-2002, 09:56 AM