problem with dynamic memory allocation

[David d'Angers]

hi group:

i've been struggling with this simple problem with dismay

i need all the file names in a directory
since the number of files in a directory is unknown, a friend here in
the group hinted me to use dynamic memory allocation: namely functions
like malloc etc

in the spirit of code re-use
i tried to find similar codes in the program "tree"
and found two wrapper functions xmalloc, xrealloc
without knowing why, i assume these wrappers are easier and safer to
use than the originals
so i wrote the following function read_image_files(), using those two
wrappers


i assume there are only 50 files in a directory,so
#define MAX_FILE_NUM 50

but when there are more than 50, my program eats up the CPU
and i don't know why

especially:
char **imagefiles = (char **)xmalloc(sizeof(char *) * MAX_FILE_NUM);
char **imagefiles = (char **)xmalloc(sizeof(char **) * MAX_FILE_NUM);
i have no idea which one to use and what the difference is



char ** read_image_files (char *dir_name)
{

char **imagefiles = (char **)xmalloc(sizeof(char *) *
MAX_FILE_NUM);

DIR *d;
struct dirent *ent;

int count = 0;

/*navigate the directory*/
while ((ent = (struct dirent *) readdir (d)))
{

count++;

/* if there's more files than predicted, add more*/
if(count > MAX_FILE_NUM)
{

imagefiles = (char
**)xrealloc(imagefiles,sizeof(char *) * (sizeof(imagefiles)+ 1));

}
imagefiles[count-1] = ent->d_name;

}
return imagefiles;
}


void *xrealloc (void *ptr, size_t size)
{
register void *value = realloc (ptr,size);
if (value == 0) {
fprintf(stderr,"tree: virtual memory exhausted.\n");
exit(1);
}
return value;
}



void *xmalloc (size_t size)
{
register void *value = malloc (size);
if (value == 0) {
fprintf(stderr,"tree: virtual memory exhausted.\n");
exit(1);
}
return value;
}

 

[Mark Bluemel]

David d'Angers wrote:
....

i assume there are only 50 files in a directory,so
#define MAX_FILE_NUM 50

but when there are more than 50, my program eats up the CPU
and i don't know why

You want space for MAX_FILE_NUM pointers to char, so this is the right
call, except that
a) the cast is unnecessary (you don't need to cast void *)
and could mask problems

b) you can simplify the decision by using the clc approved form :-
char **imagefiles = xmalloc(sizeof *imagefiles * MAX_FILE_NUM);

char ** read_image_files (char *dir_name)
{

char **imagefiles = (char **)xmalloc(sizeof(char *) *
MAX_FILE_NUM);

You want space for MAX_FILE_NUM pointers to char, so this is the right
call, except that
a) the cast is unnecessary (you don't need to cast void *)
and could mask problems

b) you can simplify the decision by using the clc approved form :-
char **imagefiles = xmalloc(sizeof *imagefiles * MAX_FILE_NUM);

DIR *d;
struct dirent *ent;

int count = 0;

/*navigate the directory*/
while ((ent = (struct dirent *) readdir (d)))
{
count++;
/* if there's more files than predicted, add more*/
if(count > MAX_FILE_NUM)
{
imagefiles = (char
**)xrealloc(imagefiles,sizeof(char *) * (sizeof(imagefiles)+ 1));

sizeof(imagefiles) is sizeof(char **) - probably 4 or 8. Not helpful.
You've just shrunk your allocated space and you probably start storing
data in all sorts of unsuitable places...

Why not use
char **imagefiles = xrealloc(sizeof *imagefiles * count);
instead?

[When you've got the basics working you could look at an approach
which didn't realloc on each file after 50, for example by doubling
the size of the allocated space on overflow]

}
imagefiles[count-1] = ent->d_name;

I don't much like the [count-1] here, it may be more "elegant" to rework
where you increment the count...

More importantly (though possibly off-topic in clc) the manual tells me
that
"The data returned by readdir() may be overwritten by subsequent calls
to readdir() for the same directory stream."

So keeping a pointer to the name returned in the structure is unlikely
to be valid...

You need to create a copy of the string and store its address in
"imagefiles".

As you're clearly on a unix-like systems, you could do this with strdup():-
imagefiles[count-1] = strdup(ent->d_name);

}
return imagefiles;
}

The xmalloc and xrealloc wrappers take a fairly blunt approach to error
handling. It may be appropriate for this particular application, but
don't assume they would be universally applicable.