8

I'm puzzled by the following difference in behaviour:

// suppose myfile.txt contains a single line with the single character 's'
    errno_t res;
    FILE* fp;
    char cmd[81];

    res = fopen_s(&fp, "D:\\myfile.txt", "rb" );
    fscanf(fp,"%80s",cmd); // cmd now contains 's/0'
    fclose(fp);

    res = fopen_s(&fp, "D:\\myfile.txt", "rb" );
    fscanf_s(fp,"%80s",cmd); // cmd now contains '/0' !
    fclose(fp);

The results do not depend in the order of call (i.e., call fscanf_s first, you'd get the empty string first). Compiled on VC++ - VS2005. Can anyone reproduce? Can anyone explain?

Thanks!

Kate Gregory
  • 18,808
  • 8
  • 56
  • 85
Ofek Shilon
  • 14,734
  • 5
  • 67
  • 101

4 Answers4

9

From the docs on fscanf_s(), http://msdn.microsoft.com/en-us/library/6ybhk9kc.aspx:

The main difference between the secure functions (with the _s suffix) and the older functions is that the secure functions require the size of each c, C, s, S and [ type field to be passed as an argument immediately following the variable. For more information, see scanf_s, _scanf_s_l, wscanf_s, _wscanf_s_l and scanf Width Specification.

And http://msdn.microsoft.com/en-us/library/w40768et.aspx:

Unlike scanf and wscanf, scanf_s and wscanf_s require the buffer size to be specified for all input parameters of type c, C, s, S, or [. The buffer size is passed as an additional parameter immediately following the pointer to the buffer or variable. For example, if reading a string, the buffer size for that string is passed as follows:

char s[10];

scanf("%9s", s, 10);

So you should call it like so:

fscanf_s(fp,"%80s",cmd, sizeof(cmd));
Michael Burr
  • 333,147
  • 50
  • 533
  • 760
6

fscanf_s (and the whole scanf_s family) requires that you pass the size of any %c, %C, %s, %S, or %[ after the buffer itself; you're omitting that argument:

fscanf_s(fp, "%80s", cmd, 81);
Michael Mrozek
  • 169,610
  • 28
  • 168
  • 175
  • 3
    This seems like exactly the wrong usage. `81` is supposed to be size of the `cmd` buffer. But is it? This is a good way to write code that crashes, even despite using a "secure" function. There should be `sizeof(cmd)` in case of reading into a static array of chars, some variable that stores the length the buffer is allocated to, or a call to size / capacity function of a given buffer. While your example is technically correct, it can be misleading for many people. – the swine Sep 02 '13 at 13:04
2

Your question is tagged C++ and you're compiling in VC++, but using fscanf? Get a std::ifstream.

std::string buffer;
std::ifstream fp("my filepath");
fp >> buffer;
Puppy
  • 144,682
  • 38
  • 256
  • 465
0

I think there is a chance of misleading result in:

fscanf_s(fp,"%80s",cmd, sizeof(cmd));

Instead it should be like:

fscanf_s(fp,"%80s",cmd, countof(cmd));

where countof(cmd) is total size of memory block allocated to cmd which must be greater than 80, sizeof(cmd) would be either 4 or 8 which will give error in the case.

Rakesh Solanki
  • 59
  • 6