0

I have a very strange error on my code, which I run in VS 2015 and eclipse:

int main(int argc, const char**argv) {
FILE *input = stdin;
FILE *output = stdout;
if(!argumentsValid(argv, argc)){
    mtmPrintErrorMessage(stderr, MTM_INVALID_COMMAND_LINE_PARAMETERS);
    return 1;
}
if(!changeIO(&input, &output, argv, argc)){
    mtmPrintErrorMessage(stderr, MTM_CANNOT_OPEN_FILE);
    return 1;
}
char buffer[MAX_LEN + 1];
Yad3Service yad3Service = yad3ServiceCreate();
if(!yad3Service){
    mtmPrintErrorMessage(stderr, MTM_OUT_OF_MEMORY);
    deallocateAndClose(input, output, &yad3Service);
}
int j = 0;
while (fgets(buffer, MAX_LEN, input) != NULL) {
    j++;
    printf("%d\n", j);
    char command_p1[10];
    strcpy(command_p1, strtok(buffer, " "));
    if (!strcmp(command_p1, "\n") || command_p1[0] == '#') continue;
    char command_p2[30];
    strcpy(command_p2 + 1, strtok(NULL, " "));
    command_p2[0] = ' ';
    char command[40];
    strcpy(command, strcat(command_p1, command_p2));
    char* command_arguments[10];
    int i = 0;
    while((command_arguments[i++] = strtok(NULL, " ")));
    Yad3ServiceResult res = command_parser(&yad3Service, command,
                                     command_arguments, output);
    if(res != YAD3_SUCCESS){
        if(res == YAD3_OUT_OF_MEMORY){
            deallocateAndClose(input, output, &yad3Service);
            mtmPrintErrorMessage(stderr, ((MtmErrorCode)((int)res)));
            return 1;
        }
        mtmPrintErrorMessage(stderr, ((MtmErrorCode)((int)res)));
    }
}
deallocateAndClose(input, output, &yad3Service);
return 0;

}

The problem which visual studio gives is in the end of the program, after the return command:

Run-Time Check Failure #2 - Stack around the variable 'command_p1' was corrupted.

and eclipse is doing something even stranger, at 34 iteration in the while, the inner fields of Yad3Service (which is a struct with allocated memory in it) cleared and the inner addresses doesn't seemed to be there (in debugging), they just disappear, and the next time I access to the inner fields of the pointer of the struct, I get Segmentation Fault.

Maybe its about the memory which is corrupted some how during the strcpy or the strtok, I dont understand what happens. Somebody ???

The strange disappearing happens after doing the line:

strcpy(command, strcat(command_p1, command_p2));
John Coleman
  • 51,337
  • 7
  • 54
  • 119
KittyT2016
  • 195
  • 9

2 Answers2

2

You have got all kinds of problems here. First, the line:

strcpy(command_p1, strtok(buffer, " "));

There are two issues here:

First, strtok() will return NULL if buffer contains an empty string, which is invalid for your strcpy().

Second, your command_p1 is declared as an array of char[10]. Are you absolutely sure that the first token in buffer can never be more than 9 characters (not including the null terminator)? This should always be checked ahead of time, or use strlcpy() (http://linux.die.net/man/3/strlcpy) if it is available on your system. Never use strcpy() when you do not know for sure the size of what you are copying. If you do not have strlcpy you can use the following common idiom:

char mybuffer[100];

/* copy as much of mysource into mybuffer as will fit. */
/* the result may NOT be null terminated               */
strncpy(mybuffer, mysource, sizeof(mybuffer));

/* make sure the string is null terminated */
mybuffer[sizeof(mybuffer)-1] = 0;

The same caveats apply for all the string handling in your program. In several places you are making assumptions about the length of strings, and you should never do that when programming in C.

Go over your program line by line thinking through each possible scenario in which what you are copying into any string buffer may turn out to be a NULL pointer or may turn out to be larger than you expect. Fix all those and your program will probably work.. if it doesn't, you'll be in a much better position to figure out what's really wrong.

little_birdie
  • 5,600
  • 3
  • 23
  • 28
1

strcat(t,s) concatenates s onto the end of t. Thus, the space allocated for t better be large enough to hold both the string that t currently holds as well as s. In your case you have

char command_p1[10];
char command_p2[30];

Hence

strcat(command_p1, command_p2)

is risking buffer overflow. The error message shows that this actually happens.

John Coleman
  • 51,337
  • 7
  • 54
  • 119