4

I am trying to use realloc function as making the array bigger as the user entered names. It gives me an error when I add 5. element, the error like : * glibc detected ./a.out: realloc(): invalid next size: 0x00000000017d2010 ** And the code is:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void){
  char **mtn = NULL;
  char x[30];
  int i = 0;

  while ( strcmp(gets(x), "finish") ){
    mtn = realloc( mtn, i*sizeof(char) );
   // mtn[i] = realloc( mtn[i], sizeof(x) ); // tried but didnt work
    mtn[i] = x;
    i++;
  }
  puts(mtn[1]);

  return 0;
}
Guray Yildirim
  • 321
  • 3
  • 7
  • 2
    You multiply by one on the first pass through, then trample. Not good. Use `realloc(mtn, (i+1) * sizeof(char *))`. Note the changed `sizeof(char *)`, too. – Jonathan Leffler Dec 22 '12 at 00:36
  • 1
    Don't use `gets()`. Don't use `gets()` without checking its return value. Come to that, you should check the return value of `realloc()` too. The `puts(mtn[1])` seems a trifle arbitrary too. You need to use `strcpy()` to copy strings; you need to allocate space for the string (as well as the pointer to the string, which you're currently doing). – Jonathan Leffler Dec 22 '12 at 00:37
  • 1
    @KerrekSB probably at the "too localized" close vote... –  Dec 22 '12 at 00:41
  • 1
    How about helping instead of being so rude @H2CO3 This is twice now I've seen you trolling the C questions, I think you need to read the rules on what that qualifies as "too localized" http://meta.stackexchange.com/questions/4818/what-questions-should-be-closed-with-reason-too-localized – Doug Molineux Dec 22 '12 at 00:42
  • Thank you for answers, I fixed it as `realloc(mtn, (i+1) * sizeof(char *))` and now it is working. For who may come across this problem, as it is mentioned in comments, there should be `strcpy(mnt[i],x)` and of course please take a look at the other comments. Thank you so much for answers. And @KerrekSB thank you, too. – Guray Yildirim Dec 22 '12 at 00:48
  • 1
    Is it your intent to have N copies of the same address in your `char*` pointer array? if not, you must make want to allocate a new buffer for the actual *string* as well. – WhozCraig Dec 22 '12 at 01:15
  • The code above did exactly what you said. Now I fixed it. Actually that was a sample code to fix my program and I made a mistake at the point which you pointed out. Now the whole program works perfectly. – Guray Yildirim Dec 22 '12 at 03:55
  • 1
    @H2CO3: Well, it's not really too localized. Rather, there are so many separate issues that all need to be addressed to fix this that I couldn't think of a helpful answer that's below three pages. The dynamic array of pointers is just the first step; then each string should probably be an array, and then you need to think of freeing the memory; then there's the unsafe `gets`... each by themselves would make a nice, tight question, but together I literally didn't know "where to begin", that's all :-) – Kerrek SB Dec 22 '12 at 12:16
  • @KerrekSB well, after all it got the "too localized" stamp; you have a point in what you wrote, though. Maybe just what I felt too localized about it is essentially the lack of research effort. –  Dec 22 '12 at 12:27

1 Answers1

3
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void)
{
    char **mtn = NULL;
    char x[30];
    int i = 0;

    /* Never use gets, it's dangerous, use fgets instead */
    while (strcmp(fgets(x, sizeof(x), stdin), "finish\n")){
        /*
        your previous realloc was realloc(mtn, 0)
        and you have to take space for <char *>
        */
        mtn = realloc(mtn, (i + 1) * sizeof(char *));
        /* always check return of xalloc */
        if (mtn == NULL) {
            perror("realloc");
            exit(EXIT_FAILURE);
        }
        /* you still need space for store x */
        mtn[i] = malloc(strlen(x) + 1);
        if (mtn[i] == NULL) {
            perror("malloc");
            exit(EXIT_FAILURE);
        }
        strcpy(mtn[i], x); /* mtn[i] = x is not valid */
        i++;
    }
    printf("%s", mtn[1]);
    /* always free xallocs in order to prevent memory leaks */
    while (i--) free(mtn[i]);
    free(mtn);
    return 0;
}
David Ranieri
  • 39,972
  • 7
  • 52
  • 94