-2

I was writing a simple program and created an array to store 4 integers. Then I used a for loop to assign them; right after that I used the gets() function to get a string; after using the gets() function the first integer on the array would always become a 0. I even printed the variable on the screen before and after the gets() to confirm.

The only thing that fixed it was dynamically allocating the array, so now I want to know if I should always allocate arrays dynamically to prevent this kind of issue?

code:

int nums[4];
int i = 0;
char symbols[3];

for(i=0;i<4;i++){
    scanf("%d", &nums[i]);
}

fflush(stdin);
gets(symbols);
calculate(nums, symbols);
Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • Writing beyond the bounds of an object is undefined, regardless whether the object has static, automatic or allocated storage duration. – EOF Mar 20 '16 at 23:51
  • 3
    No, you shouldn't always dynamically allocate arrays. What you should always do is learn why you should never use gets(), it's obsolete, and why. Also learn about some interesting concepts called "buffer overrun", "memory corruption", and "undefined behavior". – Sam Varshavchik Mar 20 '16 at 23:51
  • How long is the string you are trying to get with `gets`? I suspect it's 3 characters. If so, then you must remember that every string must be terminated by a special character, which is `\0`, so the real length of your string must be 4, not 3. If you write 4 characters where you have place for 3 you will write beyond the end of `symbols`, and that is Undefined Behaviour. It could be that after `symbols` there's `nums`, and that's why its first element becomes a 0. – Fabio says Reinstate Monica Mar 20 '16 at 23:52
  • 1
    It doesn't matter whether the array is allocated locally or dynamically. If you type more characters than you've allocated space for (including the null terminator), you get undefined behavior. You should use `fgets` so you can specify the max number of characters to read, so it can't overflow. – Barmar Mar 20 '16 at 23:53
  • @iharob: http://pubs.opengroup.org/onlinepubs/9699919799/functions/fflush.html: *For a stream open for reading, if the file is not already at EOF, and the file is one capable of seeking, the file offset of the underlying open file description shall be set to the file position of the stream, and any characters pushed back onto the stream by ungetc() or ungetwc() that have not subsequently been read from the stream shall be discarded (without further changing the file offset).* – EOF Mar 20 '16 at 23:54
  • 1
    @SamVarshavchik: Actually `gets` is not standard (anymore). – too honest for this site Mar 20 '16 at 23:56
  • @iharob: Oh, I know C leaves this undefined. But your claim that `fflush(stdin)` is windows-specific is wrong. – EOF Mar 20 '16 at 23:56
  • @EOF you are 100% right. I shouldn't state that because it's false. I will delete the comment. – Iharob Al Asimi Mar 20 '16 at 23:57
  • Note that the POSIX behaviour is for a file that is capable of seeking; terminals are not capable of seeking. You could look at [Using `fflush(stdin)`](http://stackoverflow.com/a/34247021/15168) for more information on the topic — not a lot more information since there isn't much more to give. What Windows does with `fflush(stdin)` is more akin to `tcflush(STDIN_FILENO, TCIFLUSH)` except that it also clears the unread data from the standard I/O buffer too. See also [How can I flush unread data from a tty input queue?](https://stackoverflow.com/questions/10938882/) – Jonathan Leffler Mar 21 '16 at 00:02
  • Note too [Why is `gets()` so dangerous that it should never be used?](http://stackoverflow.com/questions/1694036/why-is-the-gets-function-dangerous-why-should-it-not-be-used) – Jonathan Leffler Mar 21 '16 at 00:10
  • It could be argued that using dynamic allocation for everything reduces the chance of a stack-smashing exploit – M.M Mar 21 '16 at 00:35
  • 1
    @M.M: At the cost of enabling heap-smashing exploits. Not a great tradeoff, though heap smashing is *pobably* a bit harder to exploit in practice. On the other hand, it can also be a great deal harder to debug. – EOF Mar 21 '16 at 01:03
  • It is so ironic to see a concern about allocating 4 `int`, yet code uses `fflush(stdin); gets(symbols);`, does not check the result of `scanf("%d", &nums[i]);`, uese `gets()` after `scanf("%d"...` , a tiny `symbols[3];` with `gets(symbols);` and no size of `nums` passed to `calculate()`. So many troubles that each in themselves warrant an SO post. – chux - Reinstate Monica Mar 21 '16 at 02:00

1 Answers1

4

No, you should not.

You should allocate arrays dynamically if you don't know their size at compile-time. If you know the size at compile-time, allocate it statically.

In both cases you should think twice about the size - e.g. if you forget about the '\0' at the end of a C-String you will end up writing in memory you didn't allocate.

In your program, the problem is that you use gets() which is unusably dangerous. It almost certainly overflowed your string, leading to undefined behaviour. In your program, the undefined behaviour manifested itself as an unexpected change to the array of integers. Using dynamic memory allocation changed where the array was stored compared to the string; it changed the undefined behaviour, but didn't fix the problem (which is that you overflowed your string buffer and invoked undefined behaviour when you did so).

Community
  • 1
  • 1
Anedar
  • 4,235
  • 1
  • 23
  • 41
  • 4
    Just to add a grey area into your answer, you should also use dynamic allocation if you know the size at compile-time, but it is _too large_ to fit on the stack. Note that allocating "statically" is not the same as "on the stack". – paddy Mar 20 '16 at 23:56