Possible Memory Leak?

This is a discussion on Possible Memory Leak? within the C Programming forums, part of the General Programming Boards category; Im having trouble finding what I belive may be a memory leak. The function Im working with seems to work ...

  1. #1
    Registered User
    Join Date
    Jun 2006
    Posts
    28

    Unhappy Possible Memory Leak?

    Im having trouble finding what I belive may be a memory leak. The function Im working with seems to work fine for several calls then it crashes. Each time I call the function, the memory used by the program increases until the function crashes, the whole program does not crash, just the function. Im still very new to C programming and this is my first attempt to work with screen capture any advice on what Im doing wrong would be great... here is the function thats giving me so much trouble.

    Code:
    int grab_screen( int client_fd, char buf[3][2048] )///////////////////////////////////////////////////////////////////////////////
    {
       // FILE *f         = NULL;
        HDC hsDC        = NULL;
        HDC hdDC        = NULL;
        HBITMAP hBMP    = NULL;
        BITMAPINFO BI;
    
        png_struct *pstr;
        png_info *pinf;
        int width, height;
        int i, j, k, u, v;
        int retval, icon;
    
        icon = atoi( buf[1] + 8 );
    
        WaitForSingleObject( shot.sem, INFINITE );
    
       /* initial data setup */
    
        if( ( hsDC = GetDC( NULL ) ) == NULL )
            return( ERR_GDI_FAILED );
    
        width  = GetDeviceCaps( hsDC, HORZRES );
        height = GetDeviceCaps( hsDC, VERTRES );
    
        if( shot.width  != width  ||
            shot.height != height )
        {
            if( shot.bytes_tn != NULL ) free( shot.bytes_tn );
            if( shot.bytes_fs != NULL ) free( shot.bytes_fs );
            if( shot.rows_tn  != NULL ) free( shot.rows_tn  );
            if( shot.rows_fs  != NULL ) free( shot.rows_fs  );
    
            retval = ERR_MALLOC_FAILED;
    
            if( ( shot.bytes_tn = malloc( (
                    width * height * 3 ) / 16 ) ) == NULL )
                goto png_exit;
    
            if( ( shot.bytes_fs = malloc(
                    width * height * 3 ) ) == NULL )
                goto png_exit;
    
            if( ( shot.rows_tn = malloc( (
                    height * sizeof( unsigned char* ) ) / 4 ) ) == NULL )
                goto png_exit;
        
            if( ( shot.rows_fs = malloc(
                    height * sizeof( unsigned char* ) ) ) == NULL )
                goto png_exit;
        
            shot.width  = width;
            shot.height = height;
    
    
    
        }
    
        /* create the destination DC & bitmap */
    
        retval = ERR_GDI_FAILED;
    
        hBMP = CreateCompatibleBitmap( hsDC, shot.width, shot.height );
    
        if( hBMP == NULL )
            goto png_exit;
    
        if( ( hdDC = CreateCompatibleDC( hsDC ) ) == NULL )
            goto png_exit;
    
        /* copy desktop pixels into bitmap, get the bits */
    
        memset( &BI, 0, sizeof( BI ) );
    
        BI.bmiHeader.biSize     = sizeof( BI.bmiHeader );
        BI.bmiHeader.biWidth    = shot.width;
        BI.bmiHeader.biHeight   = shot.height;
        BI.bmiHeader.biPlanes   = 1;
        BI.bmiHeader.biBitCount = 24;
    
        if( SelectObject( hdDC, hBMP ) == NULL )
            goto png_exit;
    
        if( BitBlt( hdDC, 0, 0, shot.width, shot.height,
                    hsDC, 0, 0, SRCCOPY ) == 0 )
            goto png_exit;
    
        if( GetDIBits( hdDC, hBMP, 0, shot.height, shot.bytes_fs,
                       &BI, DIB_RGB_COLORS ) != shot.height )
            goto png_exit;
    
    
        /* setup the png data row pointers */
    
        if( icon )
        {
            int B, G, R;
            width  /= 4;
            height /= 4;
    
           // shrink the image to 25% of its size 
    
            for( j = 0, k = height - 1; j < height; j++, k-- )
            {
                for( i = 0; i < width * 3; i +=3 )
                {
                    B = G = R = 0;
    
                    for( v = j * 4; v < (j + 1) * 4; v++ )
                    {
                        for( u = i * 4; u < (i + 3) * 4; u += 3 )
                        {
                            B += shot.bytes_fs[    u + v * shot.width * 3];
                            G += shot.bytes_fs[1 + u + v * shot.width * 3];
                            R += shot.bytes_fs[2 + u + v * shot.width * 3];
                        }
                    }
    
                    shot.bytes_tn[    i + j * width * 3] = B / 16;
                    shot.bytes_tn[1 + i + j * width * 3] = G / 16;
                    shot.bytes_tn[2 + i + j * width * 3] = R / 16;
                 }
    
    	    shot.rows_tn[j] = shot.bytes_tn + k * width * 3;
            }
    
        }
        else
            for( j = 0, k = height - 1; j < height; j++, k-- )
    	    shot.rows_fs[j] = shot.bytes_fs + k * width * 3;
    
        /* finally write the png */
    
        if( send( client_fd, "HTTP/1.0 200 OK\r\nContent-Type: "
                             "image/png\r\n\r\n", 44, 0 ) != 44 )
            return( ERR_SEND_FAILED );
    
        pstr = png_create_write_struct( PNG_LIBPNG_VER_STRING, 0, 0, 0 );
        pinf = png_create_info_struct( pstr );
    
        png_set_IHDR( pstr, pinf, width, height, 8,
                      PNG_COLOR_TYPE_RGB, 
                      PNG_INTERLACE_NONE,
                      PNG_COMPRESSION_TYPE_BASE,
                      PNG_FILTER_TYPE_BASE );
    
        png_set_bgr( pstr );
    
        png_set_write_fn( pstr, (void *) client_fd,
                          user_write_data, user_flush_data );
    
        png_write_info( pstr, pinf );
        png_write_image( pstr, ( icon ) ? shot.rows_tn : shot.rows_fs );
        png_write_end( pstr, pinf );
        png_destroy_write_struct( &pstr, &pinf );
    
        retval = 0;
    	        
    
    png_exit:
    
        if( hsDC != NULL );
            ReleaseDC( NULL, hsDC );
        
        if( hdDC != NULL );
            ReleaseDC( NULL, hdDC );
    
        if( hBMP != NULL )
            DeleteObject( hBMP ); 
    
        ReleaseSemaphore( shot.sem, 1, NULL );
    
        return( retval );
    }

  2. #2
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    The first thing I noticed is that every time you encounter an error you goto png_exit, like so:
    Code:
        if( hBMP == NULL )
            goto png_exit;
    Except for here:
    Code:
        if( send( client_fd, "HTTP/1.0 200 OK\r\nContent-Type: "
                             "image/png\r\n\r\n", 44, 0 ) != 44 )
            return( ERR_SEND_FAILED );
    If send() failed, you would skip over your resource-freeing code and undoubtable cause a memory leak or two.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  3. #3
    Captain Crash brewbuck's Avatar
    Join Date
    Mar 2007
    Location
    Portland, OR
    Posts
    7,274
    Quote Originally Posted by g1i7ch View Post
    the whole program does not crash, just the function.
    Very odd thing to say. What does it mean? How can "just the function" crash? I think you're using some new-fangled definition of "crash".

  4. #4
    Registered User
    Join Date
    Jun 2006
    Posts
    28
    Very odd thing to say. What does it mean? How can "just the function" crash? I think you're using some new-fangled definition of "crash".
    Yes, odd it is, I was trying to be brief and convey my point...didnt think Iwould be haggled for my frivolous use of the word : ) . To be exact the function stops operating correctly.

    Thanks for pointing out the oversight, dwks.
    Ill fix this and see if thats the main cause.(edit- still no luck, After making 10-15 successful calls on grab_screen, the function quits returning the image. )
    Last edited by g1i7ch; 05-24-2007 at 03:30 PM. Reason: no need 2 dbl post

  5. #5
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    It stops working correctly? Can you fire up a debugger and figure out where it stops working? Or perhaps just use printf()s until you nail down the problem area?

    Are you passing the same arguments to this function? Are the global variables it accesses in the same state? Is the function returning an error value?

    I noticed something else:
    Code:
        if( shot.width  != width  ||
            shot.height != height )
        {
            if( shot.bytes_tn != NULL ) free( shot.bytes_tn );
            if( shot.bytes_fs != NULL ) free( shot.bytes_fs );
            if( shot.rows_tn  != NULL ) free( shot.rows_tn  );
            if( shot.rows_fs  != NULL ) free( shot.rows_fs  );
    
            retval = ERR_MALLOC_FAILED;
    
            if( ( shot.bytes_tn = malloc( (
                    width * height * 3 ) / 16 ) ) == NULL )
                goto png_exit;
    
            if( ( shot.bytes_fs = malloc(
                    width * height * 3 ) ) == NULL )
                goto png_exit;
    
            if( ( shot.rows_tn = malloc( (
                    height * sizeof( unsigned char* ) ) / 4 ) ) == NULL )
                goto png_exit;
        
            if( ( shot.rows_fs = malloc(
                    height * sizeof( unsigned char* ) ) ) == NULL )
                goto png_exit;
        
            shot.width  = width;
            shot.height = height;
    
    
    
        }
    If one of those malloc()s failed, some of the members of shot would be freed, but not set to NULL. So if you called this function again, because they are not NULL, it would try to free them again.

    The best way to fix that would be to set them all to NULL after freeing them, or just free them just before the malloc() calls.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  6. #6
    Registered User
    Join Date
    Jun 2006
    Posts
    28
    The best way to fix that would be to set them all to NULL after freeing them, or just free them just before the malloc() calls.
    Tried both suggestions and still no luck.
    Debugger gives only one hintThe thread has exited with code 10 (0xA).)
    but I couldnt find any helpful info on it...I am sure this function is where the problem is, maybe a buf overflow somewhere...Im just not seeing it.
    thanks for all the help dwks

  7. #7
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,048
    You did mean to divide by 4 and 16 respectively?
    Code:
            if( ( shot.bytes_tn = malloc( (
                    width * height * 3 ) / 16 ) ) == NULL )
                goto png_exit;
    
            if( ( shot.bytes_fs = malloc(
                    width * height * 3 ) ) == NULL )
                goto png_exit;
    
            if( ( shot.rows_tn = malloc( (
                    height * sizeof( unsigned char* ) ) / 4 ) ) == NULL )
                goto png_exit;
        
            if( ( shot.rows_fs = malloc(
                    height * sizeof( unsigned char* ) ) ) == NULL )
                goto png_exit;
    It looks like you access [1 + height-1 + height-1 * width * 3], which is probably beyond the boundaries of width * height * 3 / 16.
    Code:
                    shot.bytes_tn[    i + j * width * 3] = B / 16;
                    shot.bytes_tn[1 + i + j * width * 3] = G / 16;
                    shot.bytes_tn[2 + i + j * width * 3] = R / 16;
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  8. #8
    Registered User
    Join Date
    Oct 2001
    Posts
    2,934
    Just out of curiosity, what are width and height when you are testing? If width*height*3 is not a multiple of 16, you could have a small problem here:
    Code:
    >        if( ( shot.bytes_tn = malloc( (
    >                width * height * 3 ) / 16 ) ) == NULL )
    And also here if height is not a multiple of 4:
    Code:
    >        if( ( shot.rows_tn = malloc( (
    >                height * sizeof( unsigned char* ) ) / 4 ) ) == NULL )
    Also does buf[1]+8 point at some string terminated with a string terminator ('\0'), that is a valid int?

  9. #9
    Registered User
    Join Date
    Jun 2006
    Posts
    28
    Yes, this must be the trouble...is there a better way to shrink the image, this is ugly code.
    many thanks dwks

  10. #10
    Woof, woof! zacs7's Avatar
    Join Date
    Mar 2007
    Location
    Australia
    Posts
    3,459
    if( shot.bytes_tn != NULL ) free( shot.bytes_tn );
    if( shot.bytes_fs != NULL ) free( shot.bytes_fs );
    if( shot.rows_tn != NULL ) free( shot.rows_tn );
    if( shot.rows_fs != NULL ) free( shot.rows_fs );
    Is redundant since freeing a NULL pointer has no effect, free does this check itself.

    Code:
    free(shot.bytes_tn);
    free(shot.bytes_fs);
    free(shot.rows_tn);
    free(shot.rows_fs);

  11. #11
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,855
    > for several calls then it crashes
    How does it crash? Details man, we need details.

    Also, if resources are being eroded, then it's entirely possible that something else in the code is leaking them, and it's only in this function that you get to notice.

    Eg. put a return; right at the start of the function. It won't save any images, but do your other tests to see if resources are being consumed or not.
    If they are, you're digging in the wrong place.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  12. #12
    Registered User
    Join Date
    Jun 2006
    Posts
    28
    Ok, here are the details.

    Each time I call grab_screen I lose 5M of memory which just happens to be about the amount needed to store the bitmap image of my screen...but its not being freed. Make 5 calls lose 25M of memory.

    After 10-15 calls on grab_screen the function returns a blank image. Once the function starts returning a blank image the program quits eating up the memory and continues to function correctly minus the grab_screen part.

    I really appreciate all help...I sure this is something stupidly simple I missed somewhere and am just not seeing/getting.

  13. #13
    Registered User
    Join Date
    Jun 2006
    Posts
    28
    ALLRIGHT! As I thought about my reply to Salems post I looked at my cleanup I left out one important thing...

    DeleteDC(hdDC );

    Im sure this is something stupidly simple I missed somewhere and am just not seeing/getting.
    I knew it.
    Thank you all for the help All is well now.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Memory leak in this case?
    By George2 in forum C++ Programming
    Replies: 3
    Last Post: 03-22-2008, 06:05 AM
  2. memory leak in the code?
    By George2 in forum C++ Programming
    Replies: 20
    Last Post: 01-13-2008, 06:50 AM
  3. Is this code memory leak free? ---> POSIX Threads
    By avalanche333 in forum C++ Programming
    Replies: 9
    Last Post: 04-13-2007, 04:19 PM
  4. Any Memory Leak Checking Tool?
    By George2 in forum C Programming
    Replies: 4
    Last Post: 06-22-2006, 12:02 AM
  5. Manipulating the Windows Clipboard
    By Johno in forum Windows Programming
    Replies: 2
    Last Post: 10-01-2002, 10:37 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21