String arrays in c

Question

I wrote a code to read files

What is wrong in the following code I am always getting last filename if I print any arrayItem

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

char **get_files()
{
    FILE *fp;
    int status;
    char file[1000];
    char **files = NULL;
    int i = 0;
    /* Open the command for reading. */
    fp = popen("ls", "r");
    if (fp == NULL) {
        printf("Failed to run command\n" );
        //exit;
    }

    while (fgets(file, sizeof(file)-1, fp) != NULL) {

        files = (char **)realloc(files, (i + 1) * sizeof(char *));
        //files[i] = (char *)malloc(sizeof(char));
        files[i] = file;
        i++;        
    }
    printf("%s", files[0]);
    return files;
}

int main()
{
char **files = NULL;
int i =0 ;
files = get_files("");

}

Answer 1

Calling popen() on 'ls' is a bad way to do this. Take a look at opendir(), readdir(), rewinddir() and closedir().

Answer 2

you should use

files[i] = strdup(file);

instead of

files[i] = file;

The second version only lets files[i] point to your reading buffer which is always the same. With the next fgets, you'll overwrite the contents of file and thus the contents of file[i] which actually point to the same location in memory. In fact, at the end, all your file[0]..file[n] will point to the same location as file does.

With strdup(..) you're allocating a new buffer and copying the contents of file there.

Answer 3

You are reusing the file array. After you've read a filename, you need to use strdup to take a copy of it, and put that copy into the files array. Otherwise, every element in files just points to the same string.

Answer 4

Your array in char file[1000] is single dimension, regardless of how you re-allocate memory (unless I'm missing something obvious). If you are reading an unknown number of files, then a linked list is probably the best way to go about this.

Answer 5

pclose is missing for your popen. popen is only POSIX, not C89/C99. No memory-alloc check in the example, its your work ;-)

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

char **get_files(char **list)
{
  FILE *fp;
  char file[1000];
  int i=1;
  /* Open the command for reading. */
  fp = popen("ls -l", "rt");
  if( !fp )
    perror("Failed to run command\n" ),exit(1);

  while( fgets(file, sizeof file , fp) ) {

    list = realloc(list, ++i * sizeof*list );
    memmove( list+1, list, (i-1)*sizeof*list);
    *list = strcpy( malloc(strlen(file)+1), file);
  }
  pclose( fp );
  return list;
}

main()
{
  char **files = get_files(calloc(1,sizeof*files)), **start=files;
  while( *files )
  {
    puts(*files);
    free(*files++);
  }
  free(start);
  return 0;
}