2

I'm trying to set up an array of strings (in C, using Linux). The array will hold 11 strings (static length). I initially had the array set up as:

char Answers[10][100];

but in my code I have a portion that calls fgets(input,sizeof(input),stdin). When this fgets() portion is called, the final element of my Answers array was being overwritten with the value of input (something about Answers' location on the stack?). So now I'm trying to "lock-in" the memory I use for my Answers array. Would I use

char Answers=calloc(11*sizeof(char));

OR

Run it through a loop--

char Answers[10];
for(c=0;c<=10;c++)
{
    Answers[c]=calloc(1*sizeof(char);
}

Also, I'll need to use atexit() to free the allocated memory when I'm done... What would be the best way to do this since I can't pass arguments inside atexit()?

atexit(Free);

void Free()
{
    free(Answers);
}

Thanks in advance!

phyrrus9
  • 1,441
  • 11
  • 26
Katarn
  • 21
  • 1
  • 1
  • 3
  • 1
    If it needs to hold 11 strings, why have you specified "10"? – Oliver Charlesworth Jun 21 '14 at 19:26
  • 1
    1) `calloc` does not "lock in" anything. 2) freeing memory before your process exits is pointless. 3) the reason your data is overwritten is you call `fgets` with the wrong arguments. – Jon Jun 21 '14 at 19:26
  • 1
    "When this fgets() portion is called, the final element of my Answers array was being overwritten with the value of input" - what you should be doing is to find out why there's overwriting and fix that, as it's almost certainly a bug in your code causing undefined behavior. – T.C. Jun 21 '14 at 19:27
  • 1
    Why only one argument to `calloc`? – haccks Jun 21 '14 at 19:31
  • "When this fgets() portion is called, the final element of my Answers array was being overwritten" - actually it wasn't; but you were reading past the end of your array. Your array has 10 rows, so when you try to read the 11th row you happened to read what was located in memory after the array. There is no problem with your existing definition of `Answers`. – M.M Jun 22 '14 at 02:41
  • @OliCharlesworth, Array formatting contains a zero index, so the Answers array in my code would be indexed (0,1,2,3,4,5,6,7,8,9,10), which makes 11. – Katarn Jun 23 '14 at 05:06
  • @Jon, how are my arguments for fgets incorrect? – Katarn Jun 23 '14 at 05:12
  • @haccks pseudo-code, pseudo-typo; In my head I was thinking (11 elements)*(char) ----> calloc(#items,size); – Katarn Jun 23 '14 at 05:13
  • @MattMcNabb, Array formatting contains a zero index, so the Answers array in my code would be indexed (0,1,2,3,4,5,6,7,8,9,10), which makes 11. – Katarn Jun 23 '14 at 05:14
  • @Katarn your code is `char Answers[10][100];`. The `10` means that there are `10` elements, the indices are `0` through `9`. If you want to access index `10` then you need to declare `11` elements. – M.M Jun 23 '14 at 05:33

1 Answers1

7

Okay, lots of misunderstandings, I guess. Most pieces of code there are incorrect. You asked for 11 strings so char Answers[10][100]; is incorrect and you should type char Answers[11][100]; instead, that was the reason why it skipped input. About the mistakes... First - calloc() has TWO parameters and not one, like malloc(), as in the following signature:

void *calloc(size_t nmemb, size_t size);

returns a void* to the area of memory allocated, first parameter is the number of elements that you'd like to allocate and second is the size of each element. Second, as typed above, it returns a POINTER, a void one, so you can't perform this piece of code correctly:

char Answers[10];
for(c=0;c<=10;c++)
{
    Answers[c] = calloc(11*sizeof(char));
}

What's the problem ? FIRST, parameters, as said. But second is the fact you made an array of chars and not CHAR POINTERS as needed. Should be this way:

char* Answers[11]; //As you requested 11 elements
for(c=0;c<=10;c++)
{
    Answers[c] = (char*) calloc(1, MY_STR_LENGTH);
}

When MY_STR_LENGTH is a constant of 100, as shown in your example. We used casting upon the void pointer, we corrected the use of calloc and the declaration of the char pointers. Now this code is correct - also, why do you use calloc? Usually there's no need to do so in a string. By the way, no need for a function when it's one line anyway. The second way to declare this is this way:

char **Answers = calloc(11, sizeof(char*));
for(i = 0 ; i < 11 ; i++)
{
   Answers[i] = calloc(1, sizeof(char)*MY_STRING_LENGTH); //Basically it is MY_STRING_LENGTH as sizeof(char) is almost always one, but it's there only for the readability of the code.
}

This is it, hope you understand. Read more about memory allocation in C here:

About malloc() About calloc() About realloc()

atw
  • 5,428
  • 10
  • 39
  • 63
Zach P
  • 1,731
  • 11
  • 16
  • [Don't cast calloc](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – M.M Jun 22 '14 at 02:41
  • Can you explain why you are allocating `11 * MY_STRING_LENGTH` bytes for each row, instead of just `MY_STRING_LENGTH` ? – M.M Jun 22 '14 at 02:42
  • @MatMcNabb knew about no need for casting calloc (I made it there only for, let's say 'logic' in the code) but I really didn't know it can hide an error, thanks for the enlightment, I corrected it in the code. And about the allocation, indeed, my bad XD it was night-time at my place so I really didn't focus that much. Corrected too. Thanks – Zach P Jun 22 '14 at 06:36
  • @user3195614, Late hour here, so I can't implement it tonight, but I'll try it first thing tomorrow. Thanks for the help! (Can't upvote, since I'm new, but consider this an early thumbs-up). – Katarn Jun 23 '14 at 05:18