0

SEE EDIT

I execute the command "ls" with popen and then I get the output of it with fgets. What I want to do is separate each file/dir name and move it into an array of strings.

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

int main(void){
    char op[BUFSIZ]; 
    char words[BUFSIZ][BUFSIZ]; 

    int i = 0, j = 0, cnt = 0;

    FILE *ls = popen("ls", "r");

    // split files and dirs names 
    fgets(op, strlen(op), ls);
    for(i=0;i<=(strlen(op));i++) {
        if (op[i] == ' ' || op[i] == '\0') {
            words[cnt][j] = '\0';
            cnt++;
            j = 0; 
        }
        else {
            words[cnt][j] = op[i];
            j++;
        }
    }
    pclose(ls);

    while(words[i]!=NULL){
        printf("%s", words[i]);
    }
    
    
}

EDIT here's the updated code. Replaced sizeof with strlen, replaced BUFSIZ, added i++ in the while loop

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

int main(void){
    char op[100]; 
    char words[100][100]; 

    int i=0, j=0, cnt=0;

    FILE *ls = popen("ls", "r");

    // split files and dirs names 
    fgets(op, sizeof(op), ls);
    printf("%s", op);
    for(i=0;i<=(strlen(op));i++) {
        if (op[i] == ' ' || op[i] == '\0') {
            words[cnt][j] = '\0';
            cnt++;
            j = 0; 
        }
        else {
            words[cnt][j] = op[i];
            j++;
        }
    }
    pclose(ls);

    while(words[i]!=NULL){
        printf("%s", words[i]);
        i++;
    }
    return 0;
}
vanax
  • 37
  • 4
  • What specific error or incorrect behaviour do you have with the code shown? – kaylum Nov 30 '21 at 20:39
  • 2
    `fgets(op, strlen(op), ls);` needs to be `fgets(op, sizeof(op), ls);` Using `strlen` results in Undefined Behaviour because `op` contains uninitialised (garbage) data at that point. – kaylum Nov 30 '21 at 20:40
  • `strchr` could be a real useful function – Support Ukraine Nov 30 '21 at 20:42
  • @kaylum I replaced strlen with sizeof and it gives segmentation fault – vanax Nov 30 '21 at 20:46
  • Well then debug it. Run your program in a debugger and find out which line of code is seg faulting. – kaylum Nov 30 '21 at 20:54
  • 1
    For starters: `while(words[i]!=NULL)` is wrong. `i` is set to be index past the `op` string and is never changed in the loop. So it's totally the wrong index to use and is also an infinete loop. – kaylum Nov 30 '21 at 20:56
  • @kaylum lol I forgot to include it, sorry. – vanax Nov 30 '21 at 20:57
  • 1
    I don't know where `BUFSIZ` is defined or what it's meant to be used for, but on [godbolt](https://godbolt.org/z/b631jx4d7) it's 8192. This means `char words[BUFSIZ][BUFSIZ];` is 64MB, far too large for the stack which is usually 8MB by default in linux I think. – yano Nov 30 '21 at 21:00
  • @yano ok now it prints something, but it looks like "�o8�E��H���". I tried printing only "op" and it's only the first file's name. Weird. – vanax Nov 30 '21 at 21:09
  • 1
    @yano: ISO C requires the macro constant [BUFSIZ](https://en.cppreference.com/w/c/io#Macro_constants) to be defined in `stdio.h`. – Andreas Wenzel Nov 30 '21 at 21:31
  • 2
    You need `i=0` before the `while` loop. But even that is not enough. `words[i]` can never be NULL as it is a 1D array. Instead use `cnt` as the max number of `words` to print. `for (i = 0; i < cnt; i++) { printf("%s", words[i]); }` – kaylum Nov 30 '21 at 21:34
  • I don't get why you need to split any string, `ls` returns an entry for each file/dir (you don't get the whole result in a single string separated by spaces like in the terminal), so just call `fgets` in a loop, also consider using dynamic memory for the array: https://ideone.com/1xShRi – David Ranieri Nov 30 '21 at 21:56
  • @DavidRanierithank you so much for your code. Could you please give a more in-depth explanation about how it works? I'm not that good in C. Thanks again. – vanax Nov 30 '21 at 22:04
  • Which part? The `fgets` is called while there are lines to read, one line for each file/dir, the `realloc` part enlarges the array for each entry using a pointer to strings, each string/entry is duplicated using `strdup`, good to say that `strdup` is not part of the standard, but it is available with all decent compilers. (it will be introduced in the next standard C2x but is not present in the current one C18) At the end it calls `free` for each entry and for the list. – David Ranieri Nov 30 '21 at 22:16

0 Answers0