Shifting All Strings Up in Array (Problem with realloc)

Question

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

Computing sizeof (char) is just adding obfuscation to the code. It is always equal to 1. — unwind
– unwind, Commented Apr 8, 2009 at 8:53

1800 INFORMATION · Accepted Answer · 2009-04-08 09:05:29Z

3

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*));
}

edited Apr 8, 2009 at 9:05

answered Apr 8, 2009 at 8:48

1800 INFORMATION

137k30 gold badges164 silver badges242 bronze badges

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

5 Comments

Alnitak Over a year ago

don't forget to multiply frag_len by sizeof(char *)

1800 INFORMATION Over a year ago

Are you joking? char is always size 1 byte

unwind Over a year ago

Or better yet, by sizeof *frags to avoid needless repetition of the type.

Alnitak Over a year ago

no, I'm not joking. You're moving a chunk of pointers to chars, not a chunk of chars.

1800 INFORMATION Over a year ago

see this is what I meant by approximating hehe

Alnitak · Accepted Answer · 2009-04-08 09:03:41Z

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.

Anthony Towns · Accepted Answer · 2009-04-08 09:06:39Z

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.

Collectives™ on Stack Overflow

Shifting All Strings Up in Array (Problem with realloc)

3 Answers 3

5 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related