Hi dears, Why this piece of code does not free the heap memory fully! I tried it but after about 10 times of calling function data_readout(1) the heap gets full. heap is 300 bytes.
void data_readout(int line) { char *Data; switch (line) { case 1:Data=dataline1(); //Data is a pointer to a char array in heap break; case 2:Data=dataline2(); break; case 3: break; } //some code //... //... //some code free(Data); } static char *dataset_i(char *OBIS,int Value, char *Unit) { char *data,*data1; sprintf(data,"%s(%u%s)",OBIS,Value,Unit); data1=calloc(strlen(data),1); strcpy(data1,data);// return data1; } static char *dataline1() { char *r; char *q; char *c1=dataset_i(OBIS_Serialnr,EEPROM_read_int(ad_Serialnr),""); char *c2=dataset_i(OBIS_Bodynr,EEPROM_read_int(ad_Bodynr),""); char *c3=dataset_i(OBIS_Pumpont,EEPROM_read_int(ad_Pumpont),"*hours"); strcpy(q,c1); strcat(q,c2); strcat(q,c3); free(c1);free(c2);free(c3); r=calloc(strlen(q),1); strcpy(r,q);// return r; }
Remember strings have a trailing NUL that must be allocated, and not counted with strlen()
Make sure not to repeatedly free() the same allocation, manage the pointer so you mark is a NULL and only free() non-NULL pointers.
Where do you allocate the q pointer?
Wrap the calloc/free functions an track the usage, make sure you don't have a leak, and you don't multiply free, or trash the memory arena.
Check also the heap chain, make sure you don't have an issue with fragmentation.
Your code is sufficiently broken as presented that it needs fixing before further assessment would be useful.
Thanks for guidance. I have used the q pointer as variable in stack that strings are copied to and the single resulted string is moved to heap as return of the function. Yes you are right this code has fragmentation issue and needs NULL check after calloc that I have to correct it.
>>I have used the q pointer as variable in stack...
The variable is on the stack (auto/local), but has no memory behind it, and is likely to have some arbitrary value upon entry/use
Right, The way I used, q points to some where in stack not heap and memory it points to also gets freed as soon as function returns a value.
It is a 32-bit pointer, it is not pointing to anything. Your code copies a string to an undefined location.
static char *dataline1() { char *r; char *q; char *c1=dataset_i(OBIS_Serialnr,EEPROM_read_int(ad_Serialnr),""); char *c2=dataset_i(OBIS_Bodynr,EEPROM_read_int(ad_Bodynr),""); char *c3=dataset_i(OBIS_Pumpont,EEPROM_read_int(ad_Pumpont),"*hours"); strcpy(q,c1); strcat(q,c2); strcat(q,c3); free(c1);free(c2);free(c3); r=calloc(strlen(q),1); strcpy(r,q);// return r; }
char q[512]; // Might do what you claim you are expecting to happen
you told me to check for non NULL return pointers, and I have seen many codes examples that do so. in case of the code below:
c1,c2,c3 separately point to some memory in heap. I need to check comparison with NULL after the definition of last pointer and if one being NULL free other allocated areas and then return unsuccessful flag. So it will be 7 different modes for being NULL.For example if c3==NULL then first free(c1) and free(c2) and return unsuccessful flag. if I add c4 and c5 to list above there will be 31 checks, am I right or I am going a bad way?
Sorry I made a mistake in my previous message I did it like below:
char *c1,*c2,*c3; c1=dataset_i(OBIS_Serialnr,EEPROM_read_int(ad_Serialnr),""); if (c1==NULL) {return 0;} c2=dataset_i(OBIS_Bodynr,EEPROM_read_int(ad_Bodynr),""); if (c2==NULL) {free(c1); return 0;} c3=dataset_i(OBIS_Pumpont,EEPROM_read_int(ad_Pumpont),"*hours"); if (c3==NULL) {free(c1);free(c2); return 0;}