Jump to content
  • Advertisement
Sign in to follow this  
cobru

[solved] Find the memory leak (C code)

This topic is 3933 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

written for some linux daemon...
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <errno.h>

/* 
   recursive create all directories for path (like mkdir -p command)
 */

int utils_mkdir (char *path)
{
  char *buf;
  char **bp;
  char *tok;
  char *workpath;
  struct stat statbuf;
  int len, worklen, toklen;

  len = strlen (path) + 1;
  buf = strdup (path);

  if (!buf)
    return 0;

  workpath = (char *) malloc (len + 1);

  if (!workpath) {
    free (buf);
    return 0;
  }

  workpath[0] = 0;
  worklen = 0;

  bp = &buf;
  while ((tok = strsep (bp, "/"))) {
    toklen = strlen (tok);
    if (toklen == 0)		// "path like // doesn't matter"
      continue;

    // append tok to the full path
    if (worklen != 0) {
      workpath[worklen] = '/';
      worklen++;
    }

    memcpy ((workpath + worklen), tok, toklen + 1);
    worklen += toklen;

    if (stat (workpath, &statbuf) < 0) { // stat failed
      if (errno == ENOENT)	// dir not found, ok to mkdir it!
      {
	if (mkdir (workpath, S_IRWXU) < 0)
	  if (errno != EEXIST) // someone else could have made it , check errno
           { 
              free (buf);
              free (workpath);
	      return 0; // can't mkdir path (check perms)
           }
      }
      else { // some other errno
        free (buf);
        free (workpath);
	return 0; // can't stat path! (check perms)
      }
    }
    else if (!S_ISDIR (statbuf.st_mode)) {
      errno = EEXIST;
      free (buf);
      free (workpath);
      return 0; // path contains an existing file, can't mkdir a file!
    }

  }

  free (buf);
  free (workpath);

  return 1; // mkdir -p worked :D
}


[Edited by - cobru on October 11, 2007 1:51:56 AM]

Share this post


Link to post
Share on other sites
Advertisement
I'm not the best C coder in the world (considering I've never used it), but I've looked over twice and couldn't see that would cause a memory leak...

Share this post


Link to post
Share on other sites
well, the daemon is running since june and it's vmSize grew from 20Mb to 80Mb in 5 months.
That's a memory leak, but a very small one (a few bytes each call to this function) which accumulates in time.
Funny enough, valgrind didn't report memory leak in june when I tested the daemon.

edit:

must be in this function, because it's the only function which allocates small ammounts of memory (a few bytes for a path) each call.

Share this post


Link to post
Share on other sites
Have you considered heap fragmentation? This can increase the memory usage of a process without any actual memory leaks.

Share this post


Link to post
Share on other sites
Quote:
Original post by ToohrVyk
Have you considered heap fragmentation? This can increase the memory usage of a process without any actual memory leaks.


yes, I did... but the paths supplied to the function (given the way I'm using it currently) are pretty much the same length (+- a few bytes) libc should be able to re-use the freed blocks.
All I malloc() are these paths and some small control structures, the big chunks of memory I use are obtained via mmap() and munmap(), that makes the memory available immediately after releasing it and doesn't cause heap fragmentation.

Share this post


Link to post
Share on other sites
What about using alloca() instead of malloc here? That allocates memory on the stack, and will be fine for small chunks of data like this. Or just use static arrays.

Share this post


Link to post
Share on other sites
Just because you "have to" (???) use C is no excuse for not organizing your code better to avoid all the trickiness in the allocation code. And even in C, you can initialize variables at the point of declaration, and don't have to put them all at the top of the function... braces still introduce new scopes.

(Oh, and what exactly is recursive about this?)


#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <errno.h>

enum { FILE, FOLDER, DNE, ERROR } fs_entry_type;

int determine_type(char* path) {
struct stat statbuf;
if (stat(path, &statbuf) < 0) {
return (errno == ENOENT) ? DNE : ERROR;
}
return S_ISDIR(statbuf.st_mode) ? FOLDER : FILE;
}

int utils_mkdir_helper(char *path, char *buf, char *workpath) {
char *tok;
int worklen = 0;

// Put in a null terminator in case of an empty path... ?
workpath[worklen] = 0;

while ((tok = strsep(&buf, "/"))) {
int toklen = strlen(tok);
if (toklen == 0) { continue; }

// append tok to the full path
if (worklen != 0) {
workpath[worklen] = '/';
worklen++;
}
// We copy the null terminator each time and overlap writes on top of
// the previous null terminators; that way the string is properly terminated
// at the end without having to do it manually... ?
memcpy ((workpath + worklen), tok, toklen + 1);
worklen += toklen;

switch (determine_type(workpath)) {
case FILE:
errno = EEXIST;
return 0;

case DNE:
if (mkdir(workpath, S_IRWXU) < 0) {
if (errno != EEXIST) { return 0; } // propagate error
}
// AND FALL THROUGH TO
case FOLDER:
break; // folder exists or created successfully (whether by us or between
// the stat() check and mkdir() call); try the next path component.

default:
return 0;
}
}
return 1; // all path components succeeded.
}


int utils_mkdir (char *path) {
int result = 0;
char* buf = strdup(path);
/* Don't cast the result from malloc()! It is supposed to work without a
cast as long as you are properly including stdlib.h.
Also, it looks like you provided space for a null terminator twice ;)
I have reproduced that here anyway... */

char* workpath = malloc(strlen(path) + 2);
if (buf && workpath) {
result = utils_mkdir_helper(path, buf, workpath);
}
if (buf) { free(buf); }
if (workpath) { free(workpath); }
return result;
}





Although I don't think you need to do all that tricky string copying. Just *find* the slashes, and temporarily replace them with nulls (switching them back at the end of the iteration).

EDIT: Actually, I don't think you have to allocate anything at all.


#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <errno.h>

enum { FILE, FOLDER, DNE, ERROR } fs_entry_type;

int determine_type(char* path) {
struct stat statbuf;
if (stat(path, &statbuf) < 0) {
return (errno == ENOENT) ? DNE : ERROR;
}
return S_ISDIR(statbuf.st_mode) ? FOLDER : FILE;
}

int utils_mkdir_helper(char *path) {
switch (determine_type(path)) {
case FILE:
errno = EEXIST;
return 0;

case DNE:
if (mkdir(path, S_IRWXU) < 0) {
if (errno != EEXIST) { return 0; } // propagate error
}
// AND FALL THROUGH TO
case FOLDER:
break; // folder exists or created successfully (whether by us or between
// the stat() check and mkdir() call); try the next path component.

default:
return 0;
}
}
return 1; // all path components succeeded.
}

int utils_mkdir(char *path) {
char *tok;
int result = 1;
while ((tok = strchr(path, '/')) && result) {
*tok = '\0';
result = utils_mkdir_helper(path);
*tok = '/';
}
return result;
}



[Edited by - Zahlman on October 10, 2007 1:30:37 PM]

Share this post


Link to post
Share on other sites
Its a pet peeve of mine when people dump their code into a forum and say "here, find my bug!".

Learn how to use a tool like Purify or Bounds Checker. Purify has been around for 15 years, its not exactly a new tool, and it is guaranteed to find every single memory abuse bug in your code. You can even get a limited time free trial and I believe BC is the same way.

I've noticed that far too often, posters on gdnet just dump there code into the forums and expect other people to find all their problems and post the corrected code. Remember that people here are not paid to be your slave; we answer other people's questions out of our own unpaid time.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!