[Chilli] [PATCH] Fix: Memory leak of opt_run()

[David Bird]

Hi,

I think it is helpful to separate your example from chilli. Basically, I
believe you are thinking that the following would leak memory:

int main() { malloc(1); }

It does, of course, but who cares as the process is terminating (and,
yes, that does clean up all it's memory). 

David

On Mon, 2010-03-08 at 13:53 +0900, Masashi Honma wrote:
> (2010/03/05 18:18), David Bird wrote:
> > No, I haven't tried it in chilli, because it doesn't make much sense.
> > The following will demonstrate my point.
> 
> I've tested your test code and gotten same result. But this code does
> not describe what I want to say. I wrote some codes to describe my
> thought. Try this patch and boot ChilliSpot with
> 
> # chilli -f -d
> 
> Index: main-opt.c
> ===================================================================
> --- main-opt.c	(revision 297)
> +++ main-opt.c	(working copy)
> @@ -78,7 +78,17 @@
>     char hostname[USERURLSIZE];
>     int numargs;
>     int ret = -1;
> +  char *p;
>   
> +  //printf("argc=%d\n", argc);
> +  printf("argv[5]=%p\n", argv[5]);
> +  printf("argv[6]=%s\n", argv[6]);
> +  p = (char *)strtol(argv[6], NULL, 16);
> +  printf("p=%p\n", p);
> +  fflush(stdout);
> +  free(p);
> +  printf("p was freed\n");
> +  fflush(stdout);
>     options_init();
>   
>     memset(&args_info, 0, sizeof(args_info));
> Index: options.c
> ===================================================================
> --- options.c	(revision 297)
> +++ options.c	(working copy)
> @@ -117,6 +117,7 @@
>   static int opt_run(int argc, char **argv, int reload) {
>     char **newargs;
>     char file[128];
> +  char messages[128];
>     int status;
>     int i;
>   
> @@ -133,10 +134,10 @@
>       return status;
>     }
>   
> -  if (!(newargs = calloc(1, sizeof(char *) * (argc + 4)))) {
> +  if (!(newargs = calloc(1, sizeof(char *) * (argc + 16)))) {
>       return -1;
>     }
> -
> +
>     for (i=1; i < argc; i++) {
>       newargs[i] = argv[i];
>     }
> @@ -144,8 +145,15 @@
>     newargs[0] = "chilli_opt";
>     newargs[i++] = "-b";
>     newargs[i++] = file;
> -  newargs[i++] = reload ? "-r" : NULL;
> +  //newargs[i++] = reload ? "-r" : NULL;
> +  newargs[i++] = (char *)newargs;
> +  printf("index=%d\n", i - 1);
> +  printf("newargs[5]=%p\n", newargs[5]);
>   
> +  sprintf(messages, "%p", newargs[5]);
> +  newargs[i++] = messages;
> +  printf("newargs[6]=[%s]\n", newargs[6]);
> +
>     log_dbg("running chilli_opt on %s", file);
>   
>     if (execv(SBINDIR "/chilli_opt", newargs) != 0) {
> 
> 
> I've gotten the result below on Linux
> 
> <stdout>
> index=5
> newargs[5]=0x9bf2028
> newargs[6]=[0x9bf2028]
> argv[5]=0xbf99beba
> argv[6]=0x9bf2028
> p=0x9bf2028
> 
> <dmesg>
> [  409.621596] chilli_opt[3284]: segfault at 9bf2024 ip b763e071 sp bf99afb8 error 4 in libc-2.10.2.so[b75ce000+141000]
> 
> This means newargs pointer address is different between "before execv"
> and "after execv". And maybe the free() of p resulted in segfault.
> (I don't know why the address of stdout is 0x9bf2028 and dmesg is 9bf2024.)
> This result can't say the p was already freed. I think you believe that p is freed
> implicitly. Can you prove the p is freed ?
> _______________________________________________
> Chilli mailing list
> Chilli at coova.org
> http://lists.coova.org/cgi-bin/mailman/listinfo/chilli