LinuxQuestions.org
Download your favorite Linux distribution at LQ ISO.
Go Back   LinuxQuestions.org > Forums > Non-*NIX Forums > Programming
User Name
Password
Programming This forum is for all programming questions.
The question does not have to be directly related to Linux and any language is fair game.

Notices



Reply
 
Search this Thread
Old 01-19-2005, 09:09 AM   #1
nyk
Member
 
Registered: Jan 2004
Location: Berne, Switzerland
Distribution: FC4, Gentoo
Posts: 112

Rep: Reputation: 15
Memory leak?!


I wrote a small program to take photos with a canon camera at certain intervals. Now, I want it to run for about a year, but after it runs for a day or two, it occupies about 250MB of memory!
But I don't know how it's possible, the program frees all the memory it used when it waiting to take the next picture... Does anyone see the memory leak in this code?
Code:
/*********************************************************
 *
 * Captures images from a CANON IXUS in remote control mode.
 * Written January 2004 by Nick Fankhauser.
 *
 * Based on CARMEN Canon module by Mike Montemerlo
 *
 ********************************************************/
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include "canon.h"
#define MAXDIG 6
 
int main(int argc, char *argv[]) {
        usb_dev_handle *ixus;FILE *ifile;
        unsigned char **image;int *image_length;
        unsigned char **thumb;int *thumb_length;
        FILE *ifp;int exist=1;
        int run=1,err,cnt=0,ch=0,interval,k;
        char fn[30],zahl[10],zahlo[10];
        if (argc<3) {printf("Syntax: ixus_capture <prefix> <interval [s]>\n");exit(1);}
        interval=atoi(argv[2]);printf("Interval: %d\n",interval);
        strcpy(fn,"test/test1.jpg");
// find resume number
        while (exist) {
                cnt++;strcpy(fn,argv[1]);strcat(fn,"/");strcat(fn,argv[1]);
                sprintf(zahl,"%d",cnt);strcpy(zahlo,"");
                for (k=0;k<MAXDIG-strlen(zahl);k++) {strcat(zahlo,"0");}
                strcat(zahlo,zahl);strcat(fn,zahlo);strcat(fn,".jpg");
                if ((ifp=fopen(fn,"r"))==NULL) {exist=0;} else {fclose(ifp);}
        }
        printf("Resuming at: %s\n",fn);cnt--;
// end of find resume number
        if (mkdir(argv[1],0777)) {printf("Directory %s exists...\n",argv[1]);}
        ixus=canon_open_camera();if (ixus==NULL) {exit(1);}
        canon_initialize_camera(ixus);canon_rcc_init(ixus);
        canon_initialize_capture(ixus,FULL_TO_PC,FLASH_OFF);
        while (run) {
                image=malloc(1000000);thumb=malloc(100000);
                image_length=malloc(sizeof(int));thumb_length=malloc(sizeof(int));
                err=canon_capture_image(ixus,thumb,thumb_length,image,image_length);
                if (!err==0) {run=0;interval=0;}
                cnt++;strcpy(fn,argv[1]);strcat(fn,"/");strcat(fn,argv[1]);
                sprintf(zahl,"%d",cnt);strcpy(zahlo,"");
                for (k=0;k<MAXDIG-strlen(zahl);k++) {strcat(zahlo,"0");}
                strcat(zahlo,zahl);strcat(fn,zahlo);strcat(fn,".jpg");
                ifile=fopen(fn,"w");
                if (ifile==NULL) {printf("Failed to create %s!\n",fn);exit(1);}
                printf("Writing %d bytes to %s (E%d)...\n",*image_length,fn,err);
                fwrite(*image,*image_length,1,ifile);fclose(ifile);
                free(image);free(image_length);free(thumb);free(thumb_length);
                for (k=0;k<interval;k++) {
                        sleep(1);
// exit if /tmp/ixusterm exists
                        if ((ifp=fopen("/tmp/ixusterm","r"))!=NULL) {run=0;k=interval;}
                }
        }
        canon_stop_capture(ixus);canon_rcc_exit(ixus);
        canon_close_camera(ixus);return 0;
}

Last edited by nyk; 01-19-2005 at 09:12 AM.
 
Old 01-19-2005, 12:20 PM   #2
jailbait
Guru
 
Registered: Feb 2003
Location: Blue Ridge Mountain
Distribution: Debian Wheezy, Debian Jessie
Posts: 7,590

Rep: Reputation: 187Reputation: 187
"if (ifile==NULL) {printf("Failed to create %s!\n",fn);exit(1);}"

If you take this exit then you have a memory leak.

----------------------------
Steve Stites
 
Old 01-19-2005, 12:35 PM   #3
jlliagre
Moderator
 
Registered: Feb 2004
Location: Outside Paris
Distribution: Solaris10, Solaris 11, Mint, OL
Posts: 9,523

Rep: Reputation: 365Reputation: 365Reputation: 365Reputation: 365
Doesn't look there's an obvious leak in your code, what about the canon library you're using ?
 
Old 01-19-2005, 12:42 PM   #4
Hko
Senior Member
 
Registered: Aug 2002
Location: Groningen, The Netherlands
Distribution: ubuntu
Posts: 2,530

Rep: Reputation: 108Reputation: 108
Quote:
Originally posted by jailbait
"if (ifile==NULL) {printf("Failed to create %s!\n",fn);exit(1);}"

If you take this exit then you have a memory leak.
While not un- true what you say here, it does not matter because you're exiting anyway.
 
Old 01-19-2005, 01:26 PM   #5
Hko
Senior Member
 
Registered: Aug 2002
Location: Groningen, The Netherlands
Distribution: ubuntu
Posts: 2,530

Rep: Reputation: 108Reputation: 108
You seem to use some canon-API I don't know about, but I think you should not declare image and thumb as char **. Also, image_length and thumb_length should not be pointers, but regular int's. Then pass the adresses (with prefix '&') to the canon-API functions.

Like this: (changed lines in red)
Code:
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include "canon.h"
#define MAXDIG 6
 
int main(int argc, char *argv[]) {
        usb_dev_handle *ixus;FILE *ifile;
        unsigned char *image;int image_length;
        unsigned char *thumb;int thumb_length;
        FILE *ifp;int exist=1;
        int run=1,err,cnt=0,ch=0,interval,k;
        char fn[30],zahl[10],zahlo[10];
        if (argc<3) {printf("Syntax: ixus_capture <prefix> <interval [s]>\n");exit(1);}
        interval=atoi(argv[2]);printf("Interval: %d\n",interval);
        strcpy(fn,"test/test1.jpg");
// find resume number
        while (exist) {
                cnt++;strcpy(fn,argv[1]);strcat(fn,"/");strcat(fn,argv[1]);
                sprintf(zahl,"%d",cnt);strcpy(zahlo,"");
                for (k=0;k<MAXDIG-strlen(zahl);k++) {strcat(zahlo,"0");}
                strcat(zahlo,zahl);strcat(fn,zahlo);strcat(fn,".jpg");
                if ((ifp=fopen(fn,"r"))==NULL) {exist=0;} else {fclose(ifp);}
        }
        printf("Resuming at: %s\n",fn);cnt--;
// end of find resume number
        if (mkdir(argv[1],0777)) {printf("Directory %s exists...\n",argv[1]);}
        ixus=canon_open_camera();if (ixus==NULL) {exit(1);}
        canon_initialize_camera(ixus);canon_rcc_init(ixus);
        canon_initialize_capture(ixus,FULL_TO_PC,FLASH_OFF);
        while (run) {
                image=malloc(1000000);thumb=malloc(100000);
                /* REMOVE image_length=malloc(sizeof(int));thumb_length=malloc(sizeof(int)); */
                err=canon_capture_image(ixus, &thumb, &thumb_length, &image, &image_length);
                if (!err==0) {run=0;interval=0;}
                cnt++;strcpy(fn,argv[1]);strcat(fn,"/");strcat(fn,argv[1]);
                sprintf(zahl,"%d",cnt);strcpy(zahlo,"");
                for (k=0;k<MAXDIG-strlen(zahl);k++) {strcat(zahlo,"0");}
                strcat(zahlo,zahl);strcat(fn,zahlo);strcat(fn,".jpg");
                ifile=fopen(fn,"w");
                if (ifile==NULL) {printf("Failed to create %s!\n",fn);exit(1);}
                printf("Writing %d bytes to %s (E%d)...\n", image_length, fn,err);
                fwrite(image, image_length, 1, ifile);fclose(ifile);
                free(image); free(thumb); /* removed free(int's) */
                for (k=0;k<interval;k++) {
                        sleep(1);
// exit if /tmp/ixusterm exists
                        if ((ifp=fopen("/tmp/ixusterm","r"))!=NULL) {run=0;k=interval;}
                }
        }
        canon_stop_capture(ixus);canon_rcc_exit(ixus);
        canon_close_camera(ixus);return 0;
}
What (I think) happens with the canon-functions, is that their prototypes have char ** and int *. But the idea is not to declare the variables you want to use for that as char ** and int *.

Why?

These canon-functions need a chunk of memory from your program to store the image in. But in case the memory-chunk you malloc()-ed is not big enough, those functions provide a mechanism to re-allocate memory to get more space (there actually is a standard realloc() library function).

When this reallocation of your memory chunk happens, the address (the pointer itself) changes (may depend on the operating system). Also, obviously the lenght changes. And from that point your program needs to work with the new pointer(s) and length's. So the canon function needs to be able to change the pointer itself, and the changed length's of the re-allocated buffers. In order to be able to change those, it needs the addresses of them: i.e. the address of your actual pointers, and the adress of the int's. That's why the prototype specifies pointer-to-pointer's and pointer-to-int's.

So, with your program, for as long as your pictures fit inside the 1 million bytes you allocated ( "image=malloc(1000000)" ), everything may run fine. But when 1000000 bytes is not enough, the canon-function will reallocate the memory and change the pointer and length accordingly. But the new pointer and int will be lost when the function returns: the pointer inside your program will still be the old one. And when you free() it, you will free the wrong memory-chunk (which might have caused segfaults as well), leaving the new memory-chunk dangling in memory, not to be recovered until your program exits....

hope this helps.

Last edited by Hko; 01-19-2005 at 01:32 PM.
 
Old 01-19-2005, 07:20 PM   #6
nyk
Member
 
Registered: Jan 2004
Location: Berne, Switzerland
Distribution: FC4, Gentoo
Posts: 112

Original Poster
Rep: Reputation: 15
Thanks a lot for your detailed reply! I will try all this tomorrow, it very late night here...

just in case you want to have a look at the canon.c from Mike Monemerlo, it's in that package:
http://www.nyk.ch/ixus_capture.tar.bz2

For a start, I'll try make the buffer twice as big and look how it affects the memory leak till tomorrow. Although the outputted file is never bigger that 1meg, it reaches 950kb, so with some overhead it could get bigger than the buffer.

Ok, tried your changed version now, it works perfectly!! Thanks a lot for explaining this to me!
I had problems understanding which function needs &, * or nothing before the ints...
Then the easiest thing was just to allocate what I thought to be enough memory, but maybe it wasn't.
So I hope now it won't memory leak and it shouldn't unless there's something wrong with the canon module.

Last edited by nyk; 01-19-2005 at 07:51 PM.
 
Old 01-20-2005, 10:29 AM   #7
Hko
Senior Member
 
Registered: Aug 2002
Location: Groningen, The Netherlands
Distribution: ubuntu
Posts: 2,530

Rep: Reputation: 108Reputation: 108
I had a look at "canon.c", and if it looks like it does not do realloc(), as I guessed, but it does the entire allocation by itself (with calloc()) ! This means you should not do any allocation yourself in your program, otherwise you'll still have a leak, as the canon-capture function will always allocate memory for you. Thus changing your pointer, and leaving the memory that you allocated hanging around.

You'll only have to free() the image and thumbnail pointers yourself, but only when there was no error. (hen there's an error the canon function will free it.)

So here the next version ( )
(note: modified from the "ixus_capture.c" on your web-site)
Code:
int main(int argc, char *argv[]) {
    usb_dev_handle *ixus;FILE *ifile;
    unsigned char *image;int image_length;
    unsigned char *thumb;int thumb_length;
    int run=1,err,cnt=0,ch=0,interval,k;
    char fn[30],zahl[10],comm[50],zahlo[10];

    if (argc<3) {printf("Syntax: ixus_capture <prefix> <interval [s]>\n");exit(1);}
    interval=atoi(argv[2]);printf("Interval: %d\n",interval);
    printf("-> Press q to quit!\n");
    ixus=canon_open_camera();if (ixus==NULL) {exit(1);}
    canon_initialize_camera(ixus);canon_rcc_init(ixus);
    canon_initialize_capture(ixus,FULL_TO_PC,FLASH_OFF);
    tcgetattr(0,&orig);new=orig;new.c_lflag&=~ICANON;
    new.c_lflag&=~ECHO;new.c_lflag&=~ISIG;
    new.c_cc[VMIN]=1;new.c_cc[VTIME]=0;
    tcsetattr(0, TCSANOW, &new);
    mkdir(argv[1],0777);
    while (run) {
        err=canon_capture_image(ixus,&thumb,&thumb_length,&image,&image_length);
        if (err < 0) {  /* Error */
            run=0;      /* Make loop stop */
        } else {
            /* Only do this part when there was no error.
             * Otherwise the free() calls may segfault.
             */
            cnt++;strcpy(fn,argv[1]);strcat(fn,"/");strcat(fn,argv[1]);
            sprintf(zahl,"%d",cnt);strcpy(zahlo,"");
            for (k=0;k<MAXDIG-strlen(zahl);k++) {strcat(zahlo,"0");}
            strcat(zahlo,zahl);strcat(fn,zahlo);strcat(fn,".jpg");
            ifile=fopen(fn,"w");
            if (ifile==NULL) {printf("Failed to create %s!\n",fn);exit(1);}
            printf("Writing %d bytes to %s (E%d)...\n",image_length,fn,err);
            fwrite(image,image_length,1,ifile);fclose(ifile);
            free(image);free(image_length);
            for (k=0;k<interval;k++) {
                sleep(1);
                if(kbhit()) {
                    ch=readch();
                    if (ch=='q') {k=interval;run=0;}
                }
            }
        }
    }
    tcsetattr(0,TCSANOW, &orig);
    canon_stop_capture(ixus);canon_rcc_exit(ixus);
    canon_close_camera(ixus);return 0;
}

Last edited by Hko; 01-20-2005 at 10:40 AM.
 
Old 01-22-2005, 02:30 PM   #8
nyk
Member
 
Registered: Jan 2004
Location: Berne, Switzerland
Distribution: FC4, Gentoo
Posts: 112

Original Poster
Rep: Reputation: 15
Thanks again for your help!

You ment to write free
free(image);free(thumb);
and not
free(image);free(image_length);
in the modifies version, I guess?

Like this it's running very well and doesn't permanently use memory.

BTW: it takes pictures here: http://balkon.mine.nu
 
Old 01-22-2005, 04:12 PM   #9
Dark_Helmet
Senior Member
 
Registered: Jan 2003
Posts: 2,786

Rep: Reputation: 369Reputation: 369Reputation: 369Reputation: 369
Interesting project, but HOLY SHNIKEYS!

I just did some basic number crunching, and you'll be chewing through your storage space in a hurry.

Assuming one picture is 640x480 @ 16 bits per pixel = 600 KiB per picture
Note: I realize the camera will take the pictures and compress them with jpeg. So this is off... unless maybe the increased size/colors offset the compression

To cover the span of a year, that would be: 1 picture per 5 minutes * 60 minutes per hour * 24 hours per day * 365 days per year = 105,120 pictures

Total storage space consumed: 600 KiB per picture * 105,120 pictures = ~60 GiB

Granted, you could probably reduce that by 1/3 if you can intelligently filter out all the night shots. I just saw that the computer had an 80 GiB hard drive, and was curious if it would be able to handle it.

BTW, in the picture, is the computer suspended in that black bag (over the rail)? That's probably not good for the fans/termperature especially if it gets hot during the summer. Just a "heads-up" in case you hadn't considered it.

Still, like I said, it's a very interesting project; homemade time-lapse photography.
 
Old 01-23-2005, 11:14 AM   #10
Hko
Senior Member
 
Registered: Aug 2002
Location: Groningen, The Netherlands
Distribution: ubuntu
Posts: 2,530

Rep: Reputation: 108Reputation: 108
Quote:
Originally posted by nyk
Thanks again for your help!
You're welcome.


Quote:
You ment to write free
free(image);free(thumb);
and not
free(image);free(image_length);
in the modifies version, I guess?
Yes. That was my mistake.

Quote:
BTW: it takes pictures here: http://balkon.mine.nu
Nice idea.
But like Dark_Helmet already said, there are going to be a very large number of files on your server. I checked the directory http://balkon.mine.nu/pics/ and stopped loading it because it's quite large already...
 
Old 01-24-2005, 08:08 PM   #11
nyk
Member
 
Registered: Jan 2004
Location: Berne, Switzerland
Distribution: FC4, Gentoo
Posts: 112

Original Poster
Rep: Reputation: 15
I hope I'll be able to handle the storage of all these images... The 80GB should strore about 500 days I guess.. I use 115megs on average each day. If not I can strore some of them on an other server at work. Or delete the night photos, but jpg compresses them to 10% of day size anyway.
And for the problems with heat I hope that there is enough ventillation for the PC in the bag. There's a quite big hole in the back where all the cable go in and out. Its right on the height of the vent, so the hot air can get out. I think it will survive there, it doesn't get much sun at where its located (the dark backjard) anyway...
And the cold is not bad for it, as far as I know, red an article about scientist installing a iridium-phone pc webcam on southpole. Humidity could be fatal, but as the hardware is constantly warm it shouldn't condensate on it too much....
 
  


Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is Off
HTML code is Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
memory leak ? os2 Programming 1 05-19-2005 02:45 PM
Memory Leak?! RoaCh Of DisCor Linux - General 9 05-07-2005 01:26 AM
memory leak mfitzpat Linux - Newbie 1 09-24-2004 03:58 PM
memory leak mdk Mandriva 1 09-17-2004 11:54 AM
Memory Leak when using memory debugging C program on SuSE SLES8 babalina Linux - Distributions 0 10-06-2003 10:39 AM


All times are GMT -5. The time now is 01:02 AM.

Main Menu
Advertisement
My LQ
Write for LQ
LinuxQuestions.org is looking for people interested in writing Editorials, Articles, Reviews, and more. If you'd like to contribute content, let us know.
Main Menu
Syndicate
RSS1  Latest Threads
RSS1  LQ News
Twitter: @linuxquestions
identi.ca: @linuxquestions
Facebook: linuxquestions Google+: linuxquestions
Open Source Consulting | Domain Registration