#117 Memory Leak

closed-rejected
Fabian Keil
bugfix (21)
5
2010-01-24
2010-01-23
Joe Rozner
No

There was a memory leak due to the vector storing the split data not being freed after being copied into the http struct.

Discussion

  • Fabian Keil
    Fabian Keil
    2010-01-23

    • status: open --> pending-rejected
     
  • Fabian Keil
    Fabian Keil
    2010-01-23

    Thanks for the suggestion.

    Can you please explain why you think there's a memory leak, how you compiled your patch, and how you tested it?

    v is a stack-local array of pointers into buf, so trying to free it is not a good idea.

    I can't even get your patch through gcc45, and even if I could, I wouldn't expect it to work:
    gcc45 -c -pipe -fstack-protector-all -ggdb -Wshadow -Wconversion -I/usr/local/include/ -pthread -Wall urlmatch.c -o urlmatch.o
    urlmatch.c: In function 'parse_http_request':
    urlmatch.c:534:4: warning: the address of 'v' will always evaluate as 'true'
    urlmatch.c:534:4: error: incompatible types when assigning to type 'char *[10]' from type 'void *'
    gmake: *** [urlmatch.o] Error 1

     
  • Fabian Keil
    Fabian Keil
    2010-01-23

    • assigned_to: nobody --> fabiankeil
     
  • Joe Rozner
    Joe Rozner
    2010-01-23

    • status: pending-rejected --> open-rejected
     
  • Joe Rozner
    Joe Rozner
    2010-01-23

    Sorry about that, while reading the code for parse_http_request I noticed that the vector was being passed to ssplit but was being statically defined and set to zero within parse_http_request which I thought was odd because ssplit wouldn't be able grow and shrink it. I looked into the ssplit function and saw that the vector indeed does not grow and simply returns an error when/if it is filled. When I came back to my computer later I finished looking through the parse_http_request, not remembering that it was a statically defined array, and thinking it was a dynamically allocated vector didn't see it being freed at the end of the function. I should have spent more time looking over the code and making sure that it was actually an issue.

    While testing the patch I must have build the original source tree because I made sure to build it and check to be sure that it at least compiled before submitting the patch. I probably should have run it as well and checked that it worked functionally as well.

     
  • Fabian Keil
    Fabian Keil
    2010-01-24

    • status: open-rejected --> closed-rejected
     
  • Fabian Keil
    Fabian Keil
    2010-01-24

    No problem.