cant find out why valgrind gives errors in my code which has array of structures

Question

I have the following code is code snippet of large code. I am trying to free an array of structures for which I have allocated memory in this code. Unfortunately I get either a seg fault or no segfault (error free) when I change specific line in the code. I am not able to find the reason why. Can somebody please have a look and point out the reason. So If I change atom[i]->natoms to atom[i]->natoms-1 in void_free_atom I get no segault but valgrind shows that heap is not completely free. Note atom[i]->natoms is a constant number.

#include "stdheader.h"
#include "Atom.h"
Atom **atominfo(FILE *fp1,size_t nbytes,char *my_string,Ljcoeff *lj, int natoms) {

                                int i;
                            Atom **atom=(Atom **) malloc(sizeof(Atom *)*natoms);
                           //    printf("%d\n",natoms);
                            for (i = 0; i < natoms; i++) {
                                        atom[i]=(Atom *) malloc(sizeof(Atom));
                                        atom[i]->natoms=natoms;
                                        atom[i]->eps=lj->eps[i];
                                        atom[i]->sigma=lj->sigma[i];      
                    getline(&my_string, &nbytes, fp1);
                    sscanf(my_string, "%d %*d %lf %lf %lf %lf\n",  &atom[i]->id, &atom[i]->x,
                            &atom[i]->y, &atom[i]->z, &atom[i]->q);
//                                      printf("%d %d %lf %lf %lf %lf\n",i+1,atom[i]->id,atom[i]->x,atom[i]->y,atom[i]->z,atom[i]->q);
                }                              
                return atom;


}





void free_atom(Atom **atom)
{
int i;
for(i=0;i< atom[i]->natoms;i++){
//printf("%d\n",atom[i]->natoms-1);
free(atom[i]);
}
free(atom);
}

Awfully fomatted! Not just the code. Learn [ask]. And don't cast the result of `malloc` & friends in C! — too honest for this site, Feb 11 '16 at 20:29
It is hard to know where the error is. It might be a corruption of your data that occurs between allocation and freeing. What is the error Valgrind reports? When you have enabled debug symbols ( `-g`) that should give you a good hint. The way that you pass in the string and string size for `getline` to the function is also suspicious. — M Oehm, Feb 11 '16 at 20:34
"Which part?" Are you kidding? Try to find matching curly braces in the code you posted without the automatic bracket matching of your text editor.. — M Oehm, Feb 11 '16 at 20:36
It's easier to tell which is not: The final brace and the first two lines of the code! That includes the other text. — too honest for this site, Feb 11 '16 at 20:37
No errors when I compile with -g. Just the seg fault. size_t nbytes = 100; char *my_string=(char *)malloc(nbytes); — Arun Srikanth Sridhar, Feb 11 '16 at 20:59
Got it !. The mistake was using "atom[i]->natoms" in void free_atom. If I use the constant number which is 14144 it woks fine and valgrind reports no errors. I am not sure why though. @MOehm do you have an answer ?. — Arun Srikanth Sridhar, Feb 11 '16 at 21:13
Sorry for not formatting properly @Olaf. Still learning the ways to post in this forum. Pardon me this time. — Arun Srikanth Sridhar, Feb 11 '16 at 21:15
Well, you've got the answer now. I didn't see the error the first time round, but that's because your design is so baroque. there's no point to store the length of a list in each element. What do you do if the length changes? You must then update all nodes. And if the length is constant, you don't have to store it, or store it once in a separate variable and pass thtat to free. — M Oehm, Feb 12 '16 at 05:59
Yup agreed. I am in the process of modifying the code. The total number of atoms remain the same in my simulations. I am passing natoms as argument to the free function. Thanks for the help I really appreciate it @Moehm — Arun Srikanth Sridhar, Feb 13 '16 at 07:56

score 0 · Accepted Answer · answered Feb 11 '16 at 21:16

0

Suppose you have 2 atoms. Then this loop:

for(i=0;i< atom[i]->natoms;i++){
    free(atom[i]);
}

will do the following:

Set i to 0.
Check that 0 < atom[0]->natoms
That's true, so run free(atom[0]).
Check that 1 < atom[1]->natoms
That's true, so run free(atom[1]).
Check that 2 < atom[2]->natoms - Oops! There is no atom[2]!
Your program probably crashes here because atom[2] contains an invalid pointer.

answered Feb 11 '16 at 21:16

user253751

57,427
7
48
90

Thanks. But the answer is atom[i]->natoms is undefined when i=0 since atom[i] is itself a pointer @MOehm – Arun Srikanth Sridhar Feb 11 '16 at 21:33
@ArunSrikanthSridhar Why would atom[0]->natoms be undefined? – user253751 Feb 11 '16 at 21:53
so for example you have the following code: int i, atom[i]->natoms, atom[i] does not recognize the default global initialization. It has to be int i=0. I do not know why. – Arun Srikanth Sridhar Feb 11 '16 at 21:59
@ArunSrikanthSridhar That's not what code there is, though. It's `int i; i = 0; atom[i]->natoms`. – user253751 Feb 11 '16 at 22:07
It gives sef fault when I try to access atom[i]->natoms after int i. For eg int i,natoms; natoms=atom[i]->natoms; printf("%d\n",natoms); – Arun Srikanth Sridhar Feb 11 '16 at 22:27
No idea why this is happening !. but it works fine when I use int i=0 and access atom[i]->natoms and use natom=atom[i]->natoms in the original code I posted (void free_atom) @immibis – Arun Srikanth Sridhar Feb 11 '16 at 22:30
I guess the answer is here "http://stackoverflow.com/questions/7975008/initializing-variables-in-c" Do you agree with this @immibis – Arun Srikanth Sridhar Feb 11 '16 at 22:34
@ArunSrikanthSridhar I can't see how that's at all relevant to your problem. If you do `int i = 0; int natoms = atom[i]->natoms;` and then use `natoms`, then that would solve your problem because you never access `atom[2]->natoms` (assuming you only have 2 atoms). It's nothing to do with `int i = 0;` – user253751 Feb 11 '16 at 23:05
Yup got it !. Thanks @immibis – Arun Srikanth Sridhar Feb 12 '16 at 00:06

cant find out why valgrind gives errors in my code which has array of structures

1 Answers1