1

I have to read from stdin some characters (i don't know how many of them, but no more than MAX) and I want to store them in an array. Is this code snippet correct?

char *c1 = (char*) malloc(MAX * sizeof(char)); //may be too much
fgets(c, MAX, stdin);
int size = strlen(c1);
char *c2 = (char*) realloc(c1, size * sizeof(char));
free(c1);

Or is there a more elegant way of determining the right amount of memory to allocate for an array when you don't know how many elements to store?

Alexandru Dinu
  • 1,159
  • 13
  • 24
  • So why you would have a int * fro c1 and c2 since you're allocating for array for char. Another thought is you could declare an static array char c1[MAX+1]; instead of calling malloc. – Kaizhe Huang Nov 28 '14 at 20:46
  • `char *c2 = (char*) realloc(c1, size * sizeof(char));free(c1);` : `c1` is alread released if `c1` and `c2` are different. `free(c1)` will be double-free because it is released. also It's not to be released when the same. – BLUEPIXY Nov 28 '14 at 21:41

1 Answers1

1

In the fgets(), you probably meant c1, not c.

You could do it like this (if you insist on using realloc()):

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

#define MAX 50

int main() {
  char *c1 = malloc(MAX * sizeof(char));
  if(!c1) {
    printf("malloc() failed\n");
    return -1;
  }
  fgets(c1, MAX, stdin);
  int size = strlen(c1);
  char* c2;
  if(c1[size - 1] == '\n') {
    c1[size - 1] = '\0';
    c2 = realloc(c1, (size) * sizeof(char));
  } else {
    c2 = realloc(c1, (size + 1) * sizeof(char));
  }
  if(!c2) {
    printf("realloc() failed\n");
    return -1;
  }
  c1 = NULL;
  printf("Input was: %s\n", c2);
  free(c2);
  return 0;
}

Here are some comments:

  1. You expect to read a string, thus you should use char*, not int*.

  2. Usually, you don't want the newline read by fgets() to be kept, thus I used c1[size - 1] = '\0';, which overwrites it. However, if the user inputs the maximum allowed characters, then there will no room for it, thus we check if a newline was really stored in our buffer.

  3. Do not cast what malloc() returns.

  4. In the realloc(), you should allocate space for the size of the string, PLUS THE NULL TERMINATOR. That's why it's now size+1. However, in the case we overwritten the newline, there is no need for that, since we have already decreased the size of the string by one, thus size would suffice.

  5. Never forget to de-allocate your memory.


However, if I were you, I would print a message warning the user to input MAX characters and avoiding the reallocation.

You see, as the ref of fgets() states, the function will:

Reads characters from stream and stores them as a C string into str until (num-1) characters have been read or either a newline or the end-of-file is reached, whichever happens first.

So, even if the user inputs more, the program will not crash.


As Paul correctly commented, it is a really good idea to use a second pointer, so that if realloc() fails, we won't lose our data.

Also note that you should't free c1, because this pointer is invalidated and should be set to NULL. c1 will likely point where c2 points, thus if we free c1, then c2 will be pointing to garbage. Or it could be that the whole block of memory is transferred somewhere else, thus c1 points to garbage, we are trying to free.

Community
  • 1
  • 1
gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • (1) There's no need to allocate `size + 1` here, since you already reduced the length of your string with `c1[size - 1] = '\0';`. (2) `fgets()` won't put a newline there if the line was larger than the buffer, so you ought to check for it before trying to remove it. (3) "There is no need for a second pointer" - there is, because `realloc()` can fail, and you'll lose your original pointer if you don't have a second. (4) There really ought to be some error checking in here to make it a good answer. – Crowman Nov 28 '14 at 22:36
  • I had thought only for the second point @PaulGriffiths, but you are right in all four. I updated. Do you like it now? Thanks for the alert though! – gsamaras Nov 28 '14 at 23:11