Segmentation fault (core dumped) error with strcpy() (suspected)

Question

I am having trouble trying to this runtime segmentation fault in this short piece of code. I suspect it has something to do with the use of system() and strcpy() in the code but as I am not experienced with this type of error, I am unsure what to do and I have not found many helpful pages so far.

The code:

#include <stdio.h>
#include <string.h>
int main(){
        char command[31], string[128];
        strcpy(string, (char *)system("grep -Po '(?<=testString\\s)\\S+' File"));
        string[strlen(string)] = '\0';
        printf("%s", string);
        return 0;
}

I am using GCC 4.7.3 to compile the program. I would really appreciate any help with this a lot.

You're casting the `int` return value of [`system`](http://linux.die.net/man/3/system) to a string... No good can come of that. — , Sep 04 '13 at 11:11
Your usage of `system` is wrong, you may want to use [popen(3)](http://man7.org/linux/man-pages/man3/popen.3.html) — Basile Starynkevitch, Sep 04 '13 at 11:13
When using a system or other API cal, you should read the documentation regarding that call to determine what it accepts and returns. `system` does not return the output of the command you are executing. It only provides a return code. If you want to capture output, you can either have the `system` call output to a file that you open and read after the call, or use pipes (`popen`, etc). — lurker, Sep 04 '13 at 11:14
Besides the `system` error, the line `string[strlen(string)] = '\0';` is completely stupid! If string doesn't contain a 0 at the end, `strlen` will search for one even past past the buffer and then you will write a 0 on an already existing 0. The statement does quite wastefully, at best nothing, at worst a segmentation fault. — Patrick Schlüter, Sep 04 '13 at 11:33
@jkrix: Just change it to `string[127] = '\0';`. Better yet, create an integer constant called `MAX_LEN` and set it equal to `127`. Might as well rename `string` to something more meaningful, like `cmdOutput` — Engineer2021, Sep 04 '13 at 11:54
I didn't bother with any of that as this piece of code was just a small test for another piece of software, even though it is a good habit. Thanks for the help to everyone who contributed here, my main code works fine :) — jkrix, Sep 04 '13 at 13:25

score 2 · Answer 1 · answered Sep 04 '13 at 11:13

system does not return char * but int. Using its return value as string - char * - most likely will give you segfault.

int system(const char *command);

RETURN VALUE The value returned is -1 on error (e.g. fork(2) failed), and the return status of the command otherwise. This latter return status is in the format specified in wait(2). Thus, the exit code of the command will be WEXITSTATUS(status). In case /bin/sh could not be executed, the exit status will be that of a command that does exit(127).

Thanks for the very quick responses! Thanks @mbratch, I'll definitely look into that :) — jkrix, Sep 04 '13 at 11:37

Harshitha K V · Accepted Answer · 2014-07-08T07:15:23.877

The system command returns -1 on error or the return status of the command otherwise.

Type casting this integer return value is causing the segmentation fault in this case.

To copy the output of the command into a buffer we could use popen which returns a file pointer FILE * from which you can read the command output.

Here is the code:

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


int main( int argc, char *argv[] )
{

  FILE *fp;
  char string[128];


  /* Open the command for reading. */
  fp = popen("grep -Po '(?<=testString\\s)\\S+' File ", "r");

  if (fp == NULL) {
        printf("Failed to run command\n" );
        exit;
  }

  /* Read the output of command */
  while (fgets(string, sizeof(string)-1, fp) != NULL) {
        printf("%s", string);
  }

  /* Close */
  pclose(fp);

  return 0;
}

Segmentation fault (core dumped) error with strcpy() (suspected)

2 Answers2