2

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("");

}
2
  • About this code sample itself: main is unterminated, return and } are missing. Function call doesn't respect the prototype. Moreover, in C, functions without argument use void inside parentheses. Commented Sep 6, 2010 at 20:39
  • I am into php, java and flex programming, for some cron jobs processing, I am writing this for the first time. Anyways thanks for your suggestion Commented Sep 7, 2010 at 6:19

5 Answers 5

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.

Sign up to request clarification or add additional context in comments.

Comments

1

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;
}

1 Comment

there is any c89/c99 standard replacement for popen, the target of this programs is to run as a cron job to process files on linux box, what is your suggestion to use? Posix standard will run on all linux boxes?
1

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

1 Comment

this I made just as example, my intension is to run some linux commands and process those outputs
1

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.

Comments

0

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.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.