Segmentation fault strcpy

Question

typedef struct Movie{
    char hallName[50];
    char movieName[50];
}movie;

I have dynamically created movie array. In processCommand function:

char *hallName;
hallName = strtok(NULL," ");

and call createHall function with this arguments.

createHall(&halllName);

then in createHall function I create movie struct element and assign values here. I want to copy hallName to Movie.hallName. I do this strcpy but I get an segmentation fault in linux. In windows this code work correctly.

strcpy(Movie.hallName, *hallName);

How can I fix this problem ?

EDIT:

int main(int argc, char *argv[])
{
     FILE *inputFile = fopen(argv[1], "r+");
     FILE *outputFile = fopen("output.txt","w+");
     char *line=NULL;
     movie *Movies = (movie*)malloc((hallNumber)*sizeof(movie));

     while(1) {
          line = readLine(inputFile);
          if (line == NULL)
               break;
          processCommand(line,outputFile,Movies,hallNumber);
     }
     free(Movies);
     closeFiles(inputFile,outputFile);
     return 1;
}

void processCommand(char *line) {

     char *hallName = NULL, *command = NULL;
     command = strtok(line, " ");

     if (strcmp(command, "CREATEHALL") == 0) {
          hallName = strtok(NULL, " ");
          createHall(&hallName);
     }
     ...
}

void createHall(char **hallName) {
    movie Movie;
    strcpy(Movie.hallName, *hallName); // problem in here
    ...
}

Answer 1

There are multiple problems in your code:

• you have a syntax error: char *hallName = NULL, char *command = NULL;.

• you do not check the return values of strtok(): it can be NULL in case of mismatch.

• you pass the address of the pointer hallName for no reason, just pass the value.

• you do not check for potential overflow: if the hallName points to a long string, strcpy() will cause a buffer overrun.

• you define movie as a typedef for struct Movie but also use Movie as the name for your local variable in createHall(). This is very confusing. Use Movie for both the struct tag and the typedef and lowercase movie for local variables.

Here is how to fix these problems:

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

typedef struct Movie {
    char hallName[50];
    char movieName[50];
} Movie;

...

void createHall(char *hallName) {
    Movie movie;
    snprintf(movie.hallName, sizeof movie.hallName, "%s", hallName);
    ...
    // do something with movie
}

void processCommand(char *line) {

    char *hallName = NULL;
    char *command = NULL;

    command = strtok(line, " ");

    if (command && strcmp(command, "CREATEHALL") == 0) {
        hallName = strtok(NULL, " ");
        if (hallName) {
            createHall(hallName);
        }
    }
}

Note that processCommand modified the array to which it receives a pointer. This is a bad API. You should try and avoid such side effects. strtok() is causing this side effect and it has other problems because it uses an internal state: avoid using this function.

---

You initially referred to strncpy in your question title, I edited that out since you do not actually use it. In fact you should never use strncpy(): its semantics are commonly misunderstood and very error prone. See this article by Bruce Dawson.