4

I have extracted the "meaning" part of my code (and also replace some line to simplify it).

I have 2 dynamic pointers, one for the current line (extracted from a file) and a second for the current token. Following this question, Free/delete strtok_r pointer before processing complete string? I wrote this :

int main(void) {
    int n = 455;  
    char *tok2, *freetok2;
    char *line, *freeline;

    line = freeline = malloc(n*sizeof(*line));
    tok2 = freetok2 = malloc(n*sizeof(*tok2));

    /* content of the file) */
    const char* file_reading =  "coucou/gniagnia/puet/";

    /* reading from the file */
    strcpy(line, file_reading);

    strtok(line, "/");
    /* get the second token of the line */
    tok2 = strtok(NULL, "/");

    fprintf(stdout, "%s \n", tok2); // print gniagnia
    fprintf(stdout, "%s \n", line); // print coucou

    /* free error */
    //free(tok2);

    /* worked, but maybe don't free "everything ?" */
    //free(line);

    free(freetok2);
    free(freeline);

    return 0;
}

But at the end, I'm not sure of what is correct or not, and I find this solution not so elegant (because of using 2 "save variables".

Is that correct ? Is there some ways to improve it ? Thanks

Edit: changed my code for this, (and it will handle all the lines of the file)

include <unistd.h>
include <stdlib.h>

int main(void) {
    char *tok2; 
    char *line; 

    /* content of the file) */
    const char* file_reading =  "coucou/gniagnia/puet/";
    const char* file_reading2 =  "blabla/dadada/";

    /* reading from the file */
    line = strdup(file_reading);

    strtok(line, "/");
    /* get the second token of the line */
    tok2 = strtok(NULL, "/");

    printf("%s \n", tok2);
    printf("%s \n", line);

    /* reading from the file */
    line = strdup(file_reading2);

    strtok(line, "/");
    /* get the second token of the line */
    tok2 = strtok(NULL, "/");

    printf("%s \n", tok2);
    printf("%s \n", line);

    free(line);

    return 0;
}
Community
  • 1
  • 1
roro
  • 131
  • 1
  • 2
  • 8

2 Answers2

5

You're not actually using the memory pointed by freetok2, you don't need to malloc anything, thus you don't need the freetok2 variable.

Saying free(line) or free(freeline) is the same in your code so you don't need the freeline at all.

Another problem is this: malloc(n*sizeof(*line));. You might as well be saying: malloc(n); because sizeof(char) is always 1. But best of all would be:

line = malloc(strlen(file_reading) + 1);
strcpy(line, file_reading);
cnicutar
  • 178,505
  • 25
  • 365
  • 392
  • You say I don't need the freetok2 variable, but if I try to free(tok2); I will get a heap error (free invalid pointer). Otherwise, I didn't put the +1 for the ntc, because I used already a int (don't know in advance the size of my string, I just know that it will be less than 300 char). – roro Feb 10 '12 at 15:39
  • @roro You don't need to `free(tok2)`. You only need to `free(line)`. – cnicutar Feb 10 '12 at 15:41
  • I was expecting something like that, but how is it possible ? I wanted to follow the strong "rules" free everything you allocate.. – roro Feb 10 '12 at 15:43
  • @roro `line = freeline = malloc()` is only ONE allocation, not two, so you will only need to free one of them. Moreover, you don't need to allocate 455 chars to `line` in order to assign file_reading to it. What if file_reading is longer than 455 characters? `strdup(file_reading)` is much better, that automatically allocates exactly enough memory. – Mr Lister Feb 10 '12 at 15:43
  • @roro *free everything you allocate* but you didn't allocate `tok2`, did you ? – cnicutar Feb 10 '12 at 15:44
  • @MrLister `strdup` is great but you need to be aware it's not standard. – cnicutar Feb 10 '12 at 15:45
  • @cnicutar Oops. In that case, `malloc(strlen(file_reading)+1)`. As long as you don't allocate a fixed amount of memory and hope it will be enough for the input. – Mr Lister Feb 10 '12 at 15:47
  • @MrLister it's ok for the line freeline pointers, I wanted to be sure that I free all the char*. The problem was more about the tok2 pointer, (that I allocate in my example, because I will assigned the result of the strtok function). But you say that I didn't need to allocate it. Which means I could use an unallocate pointer or that strtok will allocate it for me. – roro Feb 10 '12 at 15:49
  • @roro `tok2` is just a pointer to a location somewhere in the line. – cnicutar Feb 10 '12 at 15:57
2

The code should be modified as follows:

int main(void) {
    int n = 455;  
    char *tok2;
    char *line;

    line = malloc(n*sizeof(*line));

    /* content of the file) */
    const char* file_reading =  "coucou/gniagnia/puet/";

    /* reading from the file */
    strcpy(line, file_reading);

    strtok(line, "/");
    /* get the second token of the line */
    tok2 = strtok(NULL, "/");

    fprintf(stdout, "%s \n", tok2); // print gniagnia
    fprintf(stdout, "%s \n", line); // print coucou

    free(line);
    return 0;
}
ciphor
  • 8,018
  • 11
  • 53
  • 70
  • Ok so I simply don't need to allocate tok2. I find it weird because it sounds like using a char* without allocating it. – roro Feb 10 '12 at 15:45