0

i have this run time error "access violation writing location " with strcpy function

Here part of my code:

else if (strcmp(sentenceRecv, "405002") == 0){
    /*winVersion[SIZE] = (char*)malloc(sizeof(tempString));*/
    system("ver >> text.txt");
    FILE *pfile = fopen("text.txt", "rt+");
    if (pfile == NULL)
    {
        printf("Couldn't open file\n");
    }
    fread(tempString, sizeof(char), SIZE - 1, pfile);
    fclose(pfile);
    pfile = fopen("text.txt", "w");
    if (pfile == NULL)
    {
        printf("Couldn't open file\n");
    }
    fputc((int)' ', pfile);
    fclose(pfile);
    /*  winVersion[SIZE] = strdup(tempString);*/
    strcpy(winVersion, tempString);
    send(ClientSocket, winVersion, sizeof(winVersion), 0);
    menuCheck = 1;
}

The error is in this line:strcpy(winVersion, tempString); and in the first lines i write:

char winVersion[SIZE];
char tempString[SIZE];
Benjamin Trent
  • 7,378
  • 3
  • 31
  • 41
Nir
  • 155
  • 1
  • 2
  • 10
  • 2
    `tempString` is not null-terminated so you cannot use it as argument to `strcpy`. Try using `fgets` instead of `fread` – M.M Aug 12 '14 at 20:11
  • Or use `char tempString[SIZE] = {0};`. – molbdnilo Aug 12 '14 at 20:20
  • @MattMcNabb Since we don't know the input data format there's no way to know if fgets is appropriate. fread returns a byte count that is being ignored, when it could be used to NUL-terminate tempString ... which is just one of the MANY problems with this code. It doesn't need NUL-termination ... why does it use strcpy instead of memcpy? Why does it copy at all? Why does it send sizeof bytes instead of the number read by fread? – Jim Balter Aug 12 '14 at 20:23
  • @JimBalter yes, the question doesn't contain sufficient info to be able to give a solid answer. However he opens the file in text mode and it's called "text.txt", which suggests that it contains text. – M.M Aug 12 '14 at 20:36

1 Answers1

2

char tempString[SIZE] = {0};

strcpy() needs a null-terminated string ('\0')

Otherwise it will just keep going until it hits a '\0' somewhere in the contiguous memory which may not belong to your program hence the access violation error.

You could use char *strncpy(char *dest, char *src, size_t n); and specify SIZE as the n number of bytes to copy. This is still somewhat unsafe because the copied strings won't be null-terminated either and could cause more problems later.

http://beej.us/guide/bgc/output/html/multipage/strcpy.html

Asics
  • 858
  • 1
  • 11
  • 20
  • thank you very much!!! i change it to: char winVersion[SIZE]=""; char tempString[SIZE]=""; – Nir Aug 12 '14 at 21:12