How to free a pointer in a structure array?

Question

I have created a structure:

struct SeqInfo
{   int Len;
    int uncLen;
    int *uncIndex;
    char *seq;
    char *seqc;
    char *seqr;};

I malloc space and initialize a structure in the function:

void initSeqInfo(struct SeqInfo *SI, char *seq, char *seqc, char *seqr){
    int lenTemp;
    int lenUnc;

    // length of the seq and uncertain
    lenTemp = strlen(seq);
    lenUnc = Num_uncertains(seqr);

    // make space
    SI->seq = (char *)malloc(sizeof(char)*lenTemp);
    SI->seqc = (char *)malloc(sizeof(char)*lenTemp);
    SI->seqr = (char *)malloc(sizeof(char)*lenTemp);
    SI->uncIndex = (int *)malloc(sizeof(int)*lenUnc);

    // init seq
    SI->Len = lenTemp;
    SI->uncLen = lenUnc;

    // init index
    Index_uncertains(seqr, SI->uncIndex);

    // init seq
    strcpy(SI->seq, seq);
    strcpy(SI->seqc, seqc);
    strcpy(SI->seqr, seqr);
}

I also define a function to free the space as follows:

// free space for structure SeqInfo
void freespace(struct SeqInfo SI){
    free(SI.seq);
    free(SI.seqc);
    free(SI.seqr);
    free(SI.uncIndex);
}

I define a pointer to the structure and initialize it as follows:

struct SeqInfo *SeqRNAs;
SeqRNAs = (struct SeqInfo*)malloc(sizeof(struct SeqInfo)*NumComb);
initSeqInfo(&SeqRNAs[0], seq10, struct10_con, struct10_req);//A1P2
initSeqInfo(&SeqRNAs[1], seq13, struct13_con, struct13_req);//A3P1
initSeqInfo(&SeqRNAs[2], seq4, struct4_con, struct4_req);//P1P4
initSeqInfo(&SeqRNAs[3], seq14, struct14_con, struct14_req);//A3P2
initSeqInfo(&SeqRNAs[4], seq17, struct17_con, struct17_req);//A4P2
initSeqInfo(&SeqRNAs[5], seq18, struct18_con, struct18_req);//A4P3

The problem occurs when I try to free the space for the defined SeqRNAs[]:

for(i = 0; i < NumComb; i++){
    freespace(SeqRNAs[i]);
}

The error is:

*** glibc detected *** ./Blocks14E: free(): invalid next size (fast): 0x0000000001123210 ***

Why am I getting this error? I am pretty sure this is the only place I free the space.

No comments yet. Perhaps because it is hard to follow disjointed snippets of code and say what is wrong. Better to post a Minimal Compilable Verifiable Example that clearly demonstrates the problem. stackoverflow.com/help/mcve Quite often, doing so will reveal the problem anyway. — Weather Vane
– Weather Vane, Commented Nov 23, 2015 at 21:36

cbranch · Accepted Answer · 2015-11-23 21:50:48Z

4

You are calling strcpy() which requires a null terminator, so you need to make sure you have room for the null terminator. Your current code does not include space for the null terminator:

lenTemp = strlen(seq);
....
SI->seq = (char *)malloc(sizeof(char)*lenTemp);
....
strcpy(SI->seq, seq);

change to:

lenTemp = strlen(seq);
....
SI->seq = (char *)malloc(sizeof(char)*(lenTemp + 1));
....
strcpy(SI->seq, seq);

same for SI->seqc and SI->seqr

edited Nov 23, 2015 at 21:50

answered Nov 23, 2015 at 21:45

cbranch

4,7792 gold badges29 silver badges27 bronze badges

Sign up to request clarification or add additional context in comments.

3 Comments

M.M Over a year ago

or, without redundant/harmful warts, SI->seq = malloc(lenTemp + 1); which seems easier to read to me.

Christopher Schneider Over a year ago

And there's no need to typecast the return of malloc.

FredK Over a year ago

Don't use (char *)malloc(sizeof(char)*lenTemp). First, don't case the result of malloc - it hides the error that occurs when you forget to include the proper header file. Second, sizeof(char) is ALWAYS 1 so you should say malloc(lenTemp+1).

ChuckCottrill · Accepted Answer · 2015-11-24 00:15:49Z

Note that you declare SeqRNAs as a struct SeqInfo* and allocate an array of SeqInfo (basically a constructor for the SeqRNAs[] array),

struct SeqInfo *SeqRNAs;
SeqRNAs = (struct SeqInfo*)malloc(sizeof(struct SeqInfo)*NumComb);

You will need to define a destructor to ensure you only perform a single free on SeqRNAs (example provided at end).

Then you call constructor/initializers for your SeqInfo[] elements,

initSeqInfo(&SeqRNAs[0], seq10, struct10_con, struct10_req);//A1P2
initSeqInfo(&SeqRNAs[1], seq13, struct13_con, struct13_req);//A3P1
initSeqInfo(&SeqRNAs[2], seq4, struct4_con, struct4_req);//P1P4
initSeqInfo(&SeqRNAs[3], seq14, struct14_con, struct14_req);//A3P2
initSeqInfo(&SeqRNAs[4], seq17, struct17_con, struct17_req);//A4P2
initSeqInfo(&SeqRNAs[5], seq18, struct18_con, struct18_req);//A4P3

Note that you define your constructor/intializer by passing the SeqInfo pointer (equivalent to C++' this), which should be a hint for the problem,

void initSeqInfo(struct SeqInfo *SI, char *seq, char *seqc, char *seqr);

You have defined a destructor for your SeqRNAs[] elements, but you pass the struct SeqInfo rather than a pointer (struct SeqInfo*) (remember, this is C),

void freespace(struct SeqInfo SI){
    free(SI.seq);
    free(SI.seqc);
    free(SI.seqr);
    free(SI.uncIndex);
}

Because we all want to be careful, I would suggest rewriting your destructor to add guards prior to calling free, and you need to pass SI as a pointer,

void freespace(struct SeqInfo* SI){
    if(!SI) return;
    if(SI->seq) free(SI->seq);
    if(SI->seqc) free(SI->seqc);
    if(SI->seqr) free(SI->seqr);
    if(SI->uncIndex) free(SI->uncIndex);
    return;
}

You probably want to declare a destructor for your SeqInfo, and you will pass each SeqRNAs[ndx] by address (use '&' to get address),

void SeqInfoDel(struct SeqInfo* SI) {
    if(!SI) return;
    int ndx;
    for(ndx = 0; ndx < NumComb; ndx++) {
        freespace(&SeqRNAs[ndx]);
    }
    free(SI);
}

And call this like so,

SeqInfoDel(SeqRNAs); SeqRNAs=NULL;

Note also that inside the SeqInfo constructor/initializer, you peform mallocs, but omit checking the return value (malloc fail will return NULL, and your later strcpy will fault). You might use strdup instead. Or build a convenience function that allocates the string of the specified size and copies enough space, ergo,

char*
strsizedup(size_t len, char* src) {
    //handle len<=0?
    char* p = malloc(sizeof(char)*(len+1));
    if(!p) return(p);
    strncpy(p,src,len); p[len]='\0';
    return(p);
}

And rewrite your SeqInfo constructor/initializer to use this convenience function.

void initSeqInfo(struct SeqInfo *SI, char *seq, char *seqc, char *seqr){
    int lenTemp;
    int lenUnc;

    // length of the seq and uncertain
    lenTemp = strlen(seq);
    lenUnc = Num_uncertains(seqr);

    SI->Len = lenTemp;
    // make space
    SI->seq = strsizedup(lenTemp,seq);
    SI->seqc = strsizedup(lenTemp,seqc);
    SI->seqr = strsizedup(lenTemp,seqr);

    SI->uncIndex = (int *)malloc(sizeof(int)*lenUnc);
    SI->uncLen = lenUnc;

    // init index
    Index_uncertains(seqr, SI->uncIndex);
}

Note that the above function still has problems when any of seq, seqc, seqr are NULL, and may have semantic errors when len(seq) != len(seqc) != len(seqr).

The freespace function was already correct. Structs may be passed by value in C. Also you have added a bunch of redundant if statements and a redundant return;
Maybe you are unaware that freeing a null pointer is well-defined and does not cause a core dump

Collectives™ on Stack Overflow

How to free a pointer in a structure array?

2 Answers 2

3 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

3 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related