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

From stackoverflow

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

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

  1800 INFORMATION : Are you joking? char is always size 1 byte

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

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

  1800 INFORMATION : oh sorry I read that wrong

  Alnitak : you've got the arguments to memmove() the wrong way around too...

  1800 INFORMATION : see this is what I meant by approximating hehe

• 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.

• 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.