CODEWITHSUNDEEP
c
Eric Conner
Eric Conner

Reputation: 10752

Shifting All Strings Up in Array (Problem with realloc)

I'm trying to write a function that shifts all the elements in an array of strings up by one.

void shift_frags(char **frags, int frag_len, int cur)
{
    int i;
    for(i = cur; i < frag_len-1; i++)
    {
        if(strlen(frags[i+1]) > strlen(frags[i]))
            frags[i] = realloc(frags[i], strlen(frags[i+1])*sizeof(char));
        strcpy(frags[i], frags[i+1]);
    }

    free(frags[frag_len-1]);
}

This is giving me the error: "realloc(): invalid next size: ..." Each array is dynamically allocated to be the size of a string read from a file. Shouldn't I be able to dynamically allocate new array sizes since my frags parameter is an array of pointers?

Thanks

Upvotes: 0

Views: 667

Answers (3)

Anthony Towns
Anthony Towns

Reputation: 2914

Your realloc is likely failing because you're not reserving a byte for the trailing NUL ('\0') character in strings -- adding a +1 to your realloc size:

    if(strlen(frags[i+1]) > strlen(frags[i]))
        frags[i] = realloc(frags[i], (strlen(frags[i+1]) + 1)*sizeof(char));
    strcpy(frags[i], frags[i+1]);

will fix that bug. The specific error you're getting is likely because one of your strings is length 0, and realloc(foo, 0) simply gives you that error on your system, or because you're writing the trailing '\0' in unallocated memory and overwriting something else important, causing corruption.

Simply rearranging pointers (frags[i] = frags[i+1], or using memmove()) is easier, quicker and stops you wasting memory, though.

Upvotes: 1

1800 INFORMATION
1800 INFORMATION

Reputation: 135355

Since as you say the array is just an array of pointers, you do not need to perform any reallocations. You just need to copy the pointers themselves. A simple call to memmove or something similar in the portion of the array is all that is required.

Something approximating this.

void shift_frags(char **frags, int frag_len, int cur)
{
  free(frags[frag_len]);
  memmove(frags+cur+1, frags+cur, (frag_len-cur) * sizeof(char*));
}

Upvotes: 3

Alnitak
Alnitak

Reputation: 339906

There's no need to free() / realloc() at all.

Your char **frags is a pointer to a list of pointers so you can just shuffle the pointer values around without creating new strings.

Just make sure you start at the far end of the list, and count backwards, or use memmove():

void shift_frags(char **frags, int frag_len, int cur)
{
    int i;

    free(frags[frag_len]); /* because otherwise this is left dangling */

    for(i = frag_len; i > cur; i--)
    {
        frags[i] = frags[i - 1];
    }
}

or:

void shift_frags(char **frags, int frag_len, int cur)
{
    int n = frag_len - cur;
    frags += cur;
    free(frags[n]);
    memmove(frags + 1, frags, n * sizeof(*frags)); /* nb: memmove(dst, src, n) */
}

NB: there's a possible off-by-one error here, it depends on the semantics of your frag_len value, and whether you know that the frag block of memory is already large enough to hold another pointer.

Upvotes: 2

Related Questions