-1

i have writen some C code to read two Numbers and one Operator:

int Numberone = 0, Numbertwo = 0;
char Op;
scanf_s("%d %c %d", &Numberone, &Op, &Numbertwo);

After the user entered his Input (5 + 2) the variables have the followeing value: Numberone = 5, Op = + and Numbertwo = 0. But that is wrong. Number two shall be 2. How can I improve my code? And why is my code wrong?

NelDav
  • 785
  • 2
  • 11
  • 30
  • What __exactly__ is your input, and what _exactly is your __actual__ output and what __exactly__ is your expected output. Please [edit] your question. – Jabberwocky Oct 12 '17 at 14:08
  • 4
    `scanf_s()` (the microsoft one) expects a buffer size for `%c`. Just don't use it, use `scanf()`. –  Oct 12 '17 at 14:10
  • 2
    `scanf_s("%d %c %d", &Numberone, &Op, &Numbertwo);` --> `scanf_s("%d %c %d", &Numberone, &Op, 1, &Numbertwo);`. See [scanf_s](https://msdn.microsoft.com/en-us/library/w40768et.aspx) – BLUEPIXY Oct 12 '17 at 14:10
  • 1
    btw, for actual user-input, `scanf()` and friends is the wrong choice anyways (but `sscanf()` on a line of input you got from `fgets()` could be fine). See my [beginners' guide away from `scanf()`](http://sekrit.de/webdocs/c/beginners-guide-away-from-scanf.html) for some info. –  Oct 12 '17 at 14:16

2 Answers2

1

You are using scanf_s (note the _s), which requires a maximum length parameter for every parameter of type %s or %c (cf, for example, scanf_s at cppreference.com):

scanf_s: Same as scanf, except that %c, %s, and %[ conversion specifiers each expect two arguments (the usual pointer and a value of type rsize_t indicating the size of the receiving array, which may be 1 when reading with a %c into a single char).

So you have too few parameters for your %c format, and it should be something like:

scanf_s("%d %c %d", &Numberone, &Op, (rsize_t) 1, &Numbertwo);
Stephan Lechner
  • 34,891
  • 4
  • 35
  • 58
  • This describes the standard version of `scanf_s()` while my crystal ball suggests OP is using the MS function ... (very similar but unfortunately not exactly the same) –  Oct 12 '17 at 14:47
0

First, you shouldn't use scanf_s(). Microsoft's compiler is telling you to use it, but it's really best to ignore this. While scanf_s() was designed to be "more secure" by requiring buffer sizes for c, s and [ conversions, it was a proprietary Microsoft extension, so using it, you're locked in to Microsoft compilers. Your actual problem is that you don't pass a buffer size in your call, scanf_s() assumes that &Numbertwo is the buffer length.

The situation is even worse since the C standard also includes a scanf_s(). The standard version expects the buffer size to be given as an element count as an rsize_t, while the Microsoft version expects a byte count as an unsigned, so they are subtly incompatible! IMHO, this whole function is unnecessary anyways, just using field widths allows you to write secure code with plain scanf().

Because scanf() isn't well-suited for recovering from parsing errors, you shouldn't use it directly for interactive input, instead read a whole line and parse that (for example with sscanf()). Your code could then look like this:

int Numberone = 0, Numbertwo = 0;
char Op;
char line[512];
if (!fgets(line, sizeof line, stdin))
{
    // error reading ...
}
if (sscanf(line, "%d %c %d", &Numberone, &Op, &Numbertwo) != 3)
{
    // error parsing ...
}

handle errors accordingly.