Saving chars to array

[ReggieBE]

I'm trying to save filenames into an array of chars.
My code seems incorrect, but can't figure out what's wrong :\

char** get_categories(void)
{
      DIR* dp;
      struct dirent* ds;
      char** cat;
      int i=1;
      
      dp = opendir("rubrieken/");
      ds = readdir(dp);
      
      while(ds!=NULL)
      {
           if ( (strcmp(ds->d_name, ".") > 0) && (strcmp(ds->d_name, "..") > 0))
           {
                realloc(cat, (i++)*sizeof(char)*strlen(ds->d_name));
                cat[i-1] = malloc(sizeof(char)*strlen(ds->d_name);
                strcpy(cat[i-1], ds->d_name);
           }
           ds = readdir(dp);
      }  
      return cat;
}

[Salem]

> if ( (strcmp(ds->d_name, ".") > 0) && (strcmp(ds->d_name, "..") > 0)
Try != 0

> realloc(cat, (i++)*sizeof(char)*strlen(ds->d_name));
1. cat is uninitialised on the first call - char**cat = NULL
2. you don't assign the result to any variable
t = realloc( cat, (i++)*sizeof(char*) );
if ( t != NULL ) cat = t;
3. the sizeof should be sizeof(char*)
4. you don't need the strlen() of a pointer in order to allocate just a pointer.

> cat[i-1] = malloc(sizeof(char)*strlen(ds->d_name);
You forgot to allocate space for the \0 as well.
cat[i-1] = malloc(sizeof(char)*(strlen(ds->d_name)+1) ) ;

[ReggieBE]

Thnx for helping me out !

2. you don't assign the result to any variable
t = realloc( cat, (i++)*sizeof(char*) );
if ( t != NULL ) cat = t;

realloc returns void, so I guess there's no use to assign it to a variable ?

I guess the error has something to do with the realloc. Can't debug, Dev-C++ refuses

[Salem]

> realloc returns void
Look again.

[ReggieBE]

This is what I have now:

char** get_categories(void)
{
      DIR* dp = NULL;
      struct dirent* ds;
      char** cat = NULL;
      int i=1;
      
      dp = opendir("rubrieken/");
      ds = readdir(dp);
      
      while(ds!=NULL)
      {
           if ( (strcmp(ds->d_name, ".") > 0) && (strcmp(ds->d_name, "..") > 0))
           {
                cat = realloc(cat, (i++)*sizeof(ds->d_name));
                cat[i-1] = malloc(sizeof(char*)*(strlen(ds->d_name)+1) );
                strcpy(cat[i-1], ds->d_name);
           }
           ds = readdir(dp);
      }  
      return cat;
}

It returns some wheird characters.

EDIT: printf("%d", sizeof(cat[i-1])); returns 4. Is that normal ?

[ReggieBE]

Ok got it. Forgot to malloc cat the first time.

Working code:

char** get_categories(void)
{
      DIR* dp = NULL;
      struct dirent* ds;
      char** cat = NULL;
      char tmp[1024];
      int i=1;
      
      dp = opendir("rubrieken/");
      ds = readdir(dp);
      
      while(ds!=NULL)
      {
           if ( (strcmp(ds->d_name, ".") > 0) && (strcmp(ds->d_name, "..") > 0))
           {
                strcpy(tmp, ds->d_name);
                if (!cat)
                   cat = malloc(sizeof(char*)*strlen(tmp));
                else
                   cat = realloc(cat, (i++)*strlen(tmp)); 
                cat[i-1] = malloc(sizeof(char*)*(strlen(tmp)+1) );
                strcpy(cat[i-1], ds->d_name);
           }
           ds = readdir(dp);
      }  
      return cat;
}

This helped me too: http://www.codecomments.com/archive2...-6-223662.html

[Salem]

You didn't listen to much did you?

[OnionKnight]

realloc() behaves like malloc() if the pointer is NULL so you don't have to do all that extra checking.

[ReggieBE]

@Salem: I did, but it didn't work when I just set cat = NULL .. when I malloced it it worked. Why is that ?

EDIT: Oooooh ok, I did cat** = NULL. But I have to stel cat* to NULL also, right ?

[ReggieBE]

2. you don't assign the result to any variable
t = realloc( cat, (i++)*sizeof(char*) );
if ( t != NULL ) cat = t;

What's t ? char** ?

[Salem]

t is a temporary pointer, which you check before assigning to cat
This is because realloc can return NULL, so assigning directly to the pointer risks a memory leak.

Two versions -

char** get_categories(void)
{
  DIR* dp = NULL;
  struct dirent* ds;
  char** cat = NULL;
  int i=1;

  dp = opendir("rubrieken/");
  while( (ds = readdir(dp)) != NULL )
  {
    if ( (strcmp(ds->d_name, ".") != 0) && (strcmp(ds->d_name, "..") != 0))
    {
      void *t = realloc( cat, (i++)*sizeof(char*) );
      if ( t != NULL ) {
        cat = t;
        cat[i-1] = malloc( sizeof(char)*(strlen(ds->d_name)+1) );
        if ( cat[i-1] != NULL ) {
          strcpy(cat[i-1], ds->d_name);
        }
      }
    }
  }
  closedir( dp );
  return cat;
}

char** get_categories(void)
{
  DIR* dp = NULL;
  struct dirent* ds;
  char** cat = NULL;
  int i=0;

  dp = opendir("rubrieken/");
  while( (ds = readdir(dp)) != NULL )
  {
    if ( (strcmp(ds->d_name, ".") != 0) && (strcmp(ds->d_name, "..") != 0))
    {
      void *t = realloc( cat, (i+1) * sizeof(cat[0]) );
      if ( t != NULL ) {
        cat = t;
        cat[i] = malloc( sizeof(cat[i][0])*(strlen(ds->d_name)+1) );
        if ( cat[i] != NULL ) {
          strcpy( cat[i], ds->d_name);
          i++;
        }
      }
    }
  }
  closedir( dp );
  return cat;
}

The second one avoids sizeof on types, but instead goes for sizeof on objects.

[ReggieBE]

Oh ok thnx !
Just one question .. I get the categories listed, but some junk comes along with it like "Categoryfile=c:\java\.."

I checked ds->d_name. That's not it. Then I declared tmp with = { '\0' } .. Still no change.

[Salem]

Maybe because you have no effective way of telling where the end of the list is, so when you print it, you just dereference junk pointers?

Or maybe that you've introduced yet another fault into the code - but since you didn't post your latest effort we can't tell.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <dirent.h>

char** get_categories(int *howmany)
{
  DIR* dp = NULL;
  struct dirent* ds;
  char** cat = NULL;
  int i=0;

  dp = opendir("/boot");
  while( (ds = readdir(dp)) != NULL )
  {
    if ( (strcmp(ds->d_name, ".") != 0) && (strcmp(ds->d_name, "..") != 0))
    {
      void *t = realloc( cat, (i+1) * sizeof(cat[0]) );
      if ( t != NULL ) {
        cat = t;
        cat[i] = malloc( sizeof(cat[i][0])*(strlen(ds->d_name)+1) );
        if ( cat[i] != NULL ) {
          strcpy( cat[i], ds->d_name);
          i++;
        }
      }
    }
  }
  closedir( dp );
  *howmany = i;
  return cat;
}

int main ( ) {
  int n;
  char **p = get_categories(&n);
  int i;
  for ( i = 0 ; i < n ; i++ ) {
    printf("%s\n", p[i] );
  }
  return 0;
}

// My output
$ ls -1 /boot
config-2.6.11-1.1369_FC4
grub
initrd-2.6.11-1.1369_FC4.img
lost+found
memtest86+-1.55.1
System.map-2.6.11-1.1369_FC4
vmlinuz-2.6.11-1.1369_FC4
xen.gz
xen-syms
$ ./a.out
xen-syms
vmlinuz-2.6.11-1.1369_FC4
config-2.6.11-1.1369_FC4
grub
lost+found
initrd-2.6.11-1.1369_FC4.img
memtest86+-1.55.1
System.map-2.6.11-1.1369_FC4
xen.gz

[ReggieBE]

Kay found it. Forgot to add \0 to the last address in substr()