Strcat result having data from last strcat call

Question

I'm saving data in two different file and using strcat to concatenate these data. Strangely, the result of my last strcat call get concat to the two string I want to concat now. It might be a bit blurry so here is the code :

...
strcat(logline,"\n");
if(logging){
    if(writeInFile(logfile,"a",logline))
         printf("    Connection logged.");
    else
         printf("    Connection couldn't be logged.");        
}

if(saving){
    char* loc = (char*) malloc(BUFSIZ);
    strcat(loc,client_ip);
    strcat(loc,"-");
    strcat(loc,server_ip);
    strcat(loc,".txt");

    if(writeInFile(loc,"a",request)){
          printf("    Connection saved.");
    }
    else{
          printf("ERROR: cannot create/open savefile %s\n",loc);
          printf("Saving set to FALSE.");
          saving = false;
    }
}

bool writeInFile(char* fileName, char* openingParam, char* content){
    if(strcmp(openingParam,"a") == 0 || strcmp(openingParam,"w") == 0){
        FILE* fptr = NULL;
        fptr = fopen(fileName,openingParam);
        if ( fptr == NULL)
        {
            printf("ERROR: cannot create/open logfile %s\n",fileName);
            return false;
        }
        fprintf(fptr,"%s",content);
        fclose(fptr);
        return true;
    }
    
    return false;
}

What's happening is that the content of logline is put at the start of loc. So a file with a hell long name is created.

EDIT :: The file is supposed to be named like 192.168.1.36-192.168.1.36.txt

but is instead named like

|--> timestamp = Sat Jan  2 20:09:24 2021
|--> remote    = 192.168.1.36
|--> local     = 192.168.1.36
|--> request   = [timeout]
|--> END
192.168.1.36-192.168.1.36.txt

|--> timestamp = Sat Jan  2 20:09:24 2021
|--> remote    = 192.168.1.36
|--> local     = 192.168.1.36
|--> request   = [timeout]
|--> END

Being the value of logline obtained via strcat.

Answer 1

The strcat function requires that the destination string really is a proper null-terminated string. Otherwise it will lead to undefined behavior.

The buffer you allocate with malloc is not initialized in any way. It's definitely not a null-terminated string.

You have four possible solutions:

1. Use strcpy instead of strcat for the first call:

   strcpy(loc,client_ip);
   

2. Initialize the buffer, for example like this:

   loc[0] = '\0';  // Terminate the buffer, making it an "empty" string
   strcat(loc,client_ip);
   

3. Call calloc instead of malloc, as that will zero the allocated memory, which is the same as setting it all to the string null-terminator:

   char* loc = calloc(BUFSIZ, 1);
   

4. Use snprintf to "print" the string into the uninitialized buffer:

   snprintf(loc, BUFSIZ, "%s-%s.txt", client_ip, server_ip);
   

Personally I recommend method 4, using snprintf.

---

There's also another problem with your code: A memory leak because you don't pass the memory you allocated to free.

Either call free(loc) before loc goes out of scope; Or makeloc an array instead:

char loc[BUFSIZ];

Making loc an array instead also means you can easily initialize it:

char loc[BUFSIZ] = { '\0' };