1

I need to pass a char * to a function, but this function really needs const?

But I'm reading it from file and it's dynamic for me:

fseek(fcode, 0, SEEK_END);
i=ftell(fcode);
square_scm = (char*) malloc(i);
//free(square_scm);
fseek(fcode, 0, SEEK_SET);
fread(square_scm, 1, i, fcode);
scheme_load_string(sc, square_scm);

So square_scm here is:

"(display 
    (string-append "Answer: " 
    (number->string (square 6.480740698407859)) "\n"))юоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюоюо"

without free :

"(display 
    (string-append "Answer: " 
    (number->string (square 6.480740698407859)) "\n"))ННээээ««««««««««««««««ю"

How can I make it like char[size], like:

"(display 
    (string-append "Answer: " 
    (number->string (square 6.480740698407859)) "\n"))"

?

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
cnd
  • 32,616
  • 62
  • 183
  • 313
  • 2
    Why are you freeing `square_scm` immediately after you allocate it? – Mysticial Jan 14 '12 at 20:21
  • oh... it was for test, I've got the tresh without free too. added to question, commented free. – cnd Jan 14 '12 at 20:24
  • 2
    In fact your problem was simply that you were allocating one `char` too few and then failing to zero-terminate your string. Nothing more prosaic than that. Using C for string handling is such pain in the proverbials! – David Heffernan Jan 14 '12 at 20:35
  • @nCdy: In this context, 'proverbial' is being used as a surrogate for a four-letter word not normally deemed acceptable on a site like SO (but, in other contexts, such as discussion of sports, it would be fine when not prefixed with 'pain in the'). You could look up [PITA](http://acronymfinder.com/PITA.html) to find an equivalent expression. – Jonathan Leffler Jan 14 '12 at 21:44

3 Answers3

4

Your free() is a disaster; you only free() the data when you've finished with it.

You should probably be error checking the return codes, at least of malloc() and fread().

The data read from the file will not be null terminated. You need to over-allocate by one byte and add the terminal NUL ('\0') after reading the data into your allocated buffer. (Note that the file might have shrunk since you opened it; pay attention to the value returned from fread(). If the file grew, you won't see the extra data, but no other damage will occur.)

You ask about const. Presumably the function square_cm() has a signature such as:

void square_cm(SomeType *sc, const char *data_string);

and you are worried that you have a char * and not a const char * in your calling code?

You do not need to be concerned; the compiler will 'add const' for you. The const is actually a promise to you, the user of the function, that the function itself will not modify your data. It can take a fixed string (eg a string constant) or a dynamically allocated string, and it doesn't matter. Where you run into problems is when you have a const char * or a string literal that you need to pass to a function that takes a char *. Here the function does not self-evidently promise to leave the string alone, which can cause problems. For example, you can't pass a string literal to strtok() because the function tries to modify the string, but the string is probably in read-only memory, which triggers problems (core dumps on Unix; undefined behaviour everywhere).

So, const in a function signature is a promise to the consumer (user) of the function not to change something. You can pass const or non-const data to that function because it won't be modified either way.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
2
  1. You are using a freed pointer, which causes undefined behaviour. You should delete or move the free() call.

  2. You aren't null terminating the string after you read it from the file. You should null terminate that string. Something like:

    square_scm[i] = 0;

    Make sure the you have allocated enough space to allow for the null terminator - that means adding a +1 to your malloc() call.

    Alternately, you can add a maximum field width to your printf() call:

    printf("%*s", i, string);

    Though you haven't shown the code where the printing happens, so that may or may not be practical.

You should probably also do some error checking.

Carl Norum
  • 219,201
  • 40
  • 422
  • 469
  • with square_scm[i] = 0; "(display (string-append "Answer: " (number->string (square 6.480740698407859)) "\n"))НН" - there is no HH in file :( – cnd Jan 14 '12 at 20:26
  • Sounds like your `ftell()` told you lies. As I mentioned - you might want to do some error checking. `fread` will return a short object count, maybe you want to update `i` in that case.... – Carl Norum Jan 14 '12 at 20:28
1

Don't free the malloced memory until you are done with it. And to answer your question, always leave space for the terminating null byte in the strings and do set the last byte to null byte.

fseek(fcode, 0, SEEK_END);
i=ftell(fcode);

//allocate for null byte too
square_scm = malloc(i + 1);

//reset the memory before using it
memset(square_scm, 0, i + 1);

fseek(fcode, 0, SEEK_SET);
fread(square_scm, 1, i, fcode);
square_scm[i] = 0; //not really required as we have already memset it
scheme_load_string(sc, square_scm);

//now you should free it, assuming it is not used anymore
free(square_scm);
Amarghosh
  • 58,710
  • 11
  • 92
  • 121