-5

I declared a character pointer, and used it for to scan a string in runtime; I don't know the number of characters that I'm going to enter, so I didn't use calloc or malloc. The program ends when it reached the line scanf("%s", NewMsg_au8). I'm using CodeBlocks 17.12 editor.

I tried hard coding one of the input case like, NewMsg_au8="0123456789ABCDEF"; — that works fine.

uint8 * NewMsg_au8;
scanf("%s",NewMsg_au8);//<== 
printf("Your entered message is: %s\n",NewMsg_au8);
return NewMsg_au8;
virolino
  • 2,073
  • 5
  • 21
Vetri
  • 7
  • 3
  • 5
    Reading into an uninitialized pointer ? Undefined behavior. Reading into a literal string ? Undefined behavior. – John3136 May 24 '19 at 03:10
  • 1
    The difference isn't as great as you'd like. See [Why the `gets()` function is too dangerous to be used — ever!](https://stackoverflow.com/questions/1694036/) for why you shouldn't use `gets()`. However, not placing a size such as `"%31s"` in the `scanf()` leaves you vulnerable to overflows. In the code, you've not shown where you allocate memory for `NewMsg_au8` to point at — that's another bug. Unlike `gets()`, the `scanf()` will skip leading white space (including newlines) and after it finds one or more other characters, it will stop at the next white space. – Jonathan Leffler May 24 '19 at 03:31
  • 1
    There is no `gets()` here. Unclear what the real question is. – chux - Reinstate Monica May 24 '19 at 03:31
  • The main difference is that `scanf` exists – M.M May 24 '19 at 04:05
  • *"I declared a character pointer, and used it for to scan a string in runtime; "* - no, you used it to invoke undefined behavior. Scanning a string with `scanf` requires viable memory on which to store the string, and you've delivered none. – WhozCraig May 24 '19 at 04:40
  • See also [How to prevent `scanf()` causing a buffer overflow in C?](https://stackoverflow.com/questions/1621394/how-to-prevent-scanf-causing-a-buffer-overflow-in-c) – Jonathan Leffler May 24 '19 at 06:03

2 Answers2

2

gets(s) and scanf("%s", s) are both unsafe and potentially incorrect because:

  • with those calls as shown, there is no way for either function to determine the maximum number of characters to store into the array pointed to by s, hence overlong input will cause a buffer overrun leading to undefined behavior.
  • in your case, it is even worse as s is an uninitialized pointer, so both functions would try a store data into a random address in memory causing undefined behavior in all cases.

gets() cannot be used safely and has been deprecated in and then removed from the C Standard.

However, scanf() can be given a limit with a numeric value between % and s:

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

char *read_string(void) {
    char buf[100];
    if (scanf("%99s", buf) == 1) {
        printf("Your entered message is: %s\n", buf);
        return strdup(buf);  /* return an allocated copy of the input string */
    } else {
        /* no input, probably at end of file */
        return NULL;
    }
}

Note how only 99 characters can be stored into the array buf to allow for the null byte terminator that marks the end of a C string. The %99s conversion specification lets scanf() store at most 100 bytes into buf, including the '\0' terminator.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • You might comment that `gets()` — or a replacement like `fgets()` or POSIX `getline()` — reads a whole line regardless of spaces within the line, whereas the `%s` format skips leading white space, including newlines, and then reads a sequence of other characters up to the next white space character — possibly reading many lines containing only white space, but not reading a whole line if there are spaces between 'words'. – Jonathan Leffler May 24 '19 at 06:10
1

That is a typical beginners error. You do not save data in pointers (with gets() or scanf()) but in buffers.

Therefore, you have 2 solutions:

  1. Use an array big enough to hold the data. You have to decide yourself what "big enough" means, according to the details of your application.

  2. Use a pointer, and then allocate memory with malloc() - the size, again, you have to decide it. Do not forget to deallocate the memory when you no longer need it.


I tried hard coding one of the input case like, NewMsg_au8="0123456789ABCDEF"; — that works fine.

That is normal, because in that case the compiler automatically allocates enough memory to hold the string.


Please always remember when working with strings: you always need to allocate an extra byte for the terminating null character - the mark of the end of the string. Otherwise, you will need to ask questions again :)

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
virolino
  • 2,073
  • 5
  • 21