Release memory problem

Hi! I had problem in the next code. In it, I want to carry out a tokenizer for getting some parameters values written in a text file.

Code:

/* Se lee el valor del parámetro Longitud del símbolo Apex */

       p_cadena=p_cadena+2;
       char *param_cpy=(char*)calloc(strlen(&(cadena[p_cadena])),sizeof(char));
       strcpy(param_cpy,&(cadena[p_cadena]));
       char *lg=strtok(param_cpy,",");
       p_cadena=p_cadena+strlen(lg);
       *result=sscanf(lg,"%f",l);       free(param_cpy);

In the character chain 'param_cpy' I allocate memory in order to copy a fragment of the text chain being descomposed. After i get the token, a want to release this memory. Howewer, when i try doing it, the program crash in execution time. The instruction causing the problem is marked in red in the code above. I don´t understand the reason which this problem occurs for. Someone could help me?

Thank you.

Fernando Gutiérrez

As this is C++ I will not say "You shouldn't cast malloc/calloc"...

Code:

calloc(strlen(&(cadena[p_cadena])),sizeof(char));

Leading question: How long is this section, and how long a string does it cope with?

Is there something in a string, that makes it take up a tiny bit more space than you'd immediately think?

--
Mats

Your calloc doesn't count the \0 at the end of the string, which will be copied (and which will trash memory when the strcpy happens).

Say
char *param_cpy=calloc(strlen(&(cadena[p_cadena]))+1,sizeof(char));

Also, using calloc instead of malloc has no advantage in this instance, since you're going to overwrite all the data with the following strcpy().

Try allocating 1 or 2 extra bytes. Maybe strcpy() is filling param_cpy and not leaving a NULL at the end, or maybe there's no NULL at the end of cadena[p_cadena] and you're overflowing the buffer?
Use strncpy() to prevent buffer overflow.

> or maybe there's no NULL at the end of cadena[p_cadena] and you're overflowing the buffer?
Then the strlen() itself would have failed.

[QUOTE=Salem;678980]Your calloc doesn't count the \0 at the end of the string, which will be copied (and which will trash memory when the strcpy happens).

Say
char *param_cpy=calloc(strlen(&(cadena[p_cadena]))+1,sizeof(char));
[QUOTE]

I've to review my code, but I think that you are right and that is the reason for that my program crashes. Thank you

Originally Posted by Salem

Also, using calloc instead of malloc has no advantage in this instance, since you're going to overwrite all the data with the following strcpy().

My question is if using malloc instead of malloc have some advantage.

calloc does more work than malloc (it is after all just malloc + memset), so unless you need that feature, it isn't worth doing IMO.

Also, calloc doesn't necessarily result in valid data, especially where floats and pointers are concerned.
http://c-faq.com/malloc/calloc.html

And once you use malloc instead of calloc, why not use new[] instead?

> calloc does more work than malloc (it is after all just malloc + memset), so unless you need that feature, it isn't
> worth doing IMO.

Not to mention that it confuses people reading your code trying to figure out where you use the initialization, only to finally realize you don't.