What's the most secure way to read a long int ?

[Julien]

Hello,

I used cppcheck to detect problems and had this :
(warning) scanf without field width limits can crash with huge input data

void read_cputime(double& cpu) {
long int c;
cpu = 0;
FILE* f = fopen(CPU_TIME, "r");
if (!f) return;
int n = fscanf(f, "%ld",&c); <-- pb detected
fclose(f);
if (n != 1) return;
cpu = c;
}

First I thought about adding a number in the format :
int n = fscanf(f, "%4ld",&c);

But I want the code to be portable (it must ok for 32 bits or 64 bits).
So what to use ? A macro ? A c++ const ? C++ internal library (cin ?) ?
other ?
would a memset (c, 0, sizeof(c)) useful before ?

Julien.

 

[Ian Collins]

Hello,

I used cppcheck to detect problems and had this :
(warning) scanf without field width limits can crash with huge input data

void read_cputime(double& cpu) {
long int c;
cpu = 0;
FILE* f = fopen(CPU_TIME, "r");
if (!f) return;
int n = fscanf(f, "%ld",&c);<-- pb detected
fclose(f);
if (n != 1) return;
cpu = c;
}

First I thought about adding a number in the format :
int n = fscanf(f, "%4ld",&c);

But I want the code to be portable (it must ok for 32 bits or 64 bits).
So what to use ? A macro ? A c++ const ? C++ internal library (cin ?) ?
other ?

Do it the C++ way:

std::ifstream f( CPU_TIME );
if (!f) return;

long c;
f >> c;

if(!f) return;

 

[Julien]

...

Do it the C++ way:

std::ifstream f( CPU_TIME );
if (!f) return;

long c;
f >> c;

if(!f) return;

Ok, cppcheck is mute about this now.

What about for this kind of code ?
time_t read_progress() {
time_t stored_secs;
FILE* f = fopen(PROGRESS_FN, "r");
if (!f) return(0);
int n = fscanf(f, "%ld",&stored_secs);
fclose(f);
if (n != 1) return(0);
else return(stored_secs);
}

It's quite the same except the variable is a struct. So f >> stored_secs
wouldn't work here.

Julien

 

[Ian Collins]

On 08/13/11 09:52 PM, Julien wrote:

Please don't snip attributions, it's rude.

I wrote:
Ok, cppcheck is mute about this now.

What about for this kind of code ?

It's horrible...

time_t read_progress() {
time_t stored_secs;
FILE* f = fopen(PROGRESS_FN, "r");
if (!f) return(0);
int n = fscanf(f, "%ld",&stored_secs);
fclose(f);
if (n != 1) return(0);
else return(stored_secs);
}

It's quite the same except the variable is a struct. So f>> stored_secs
wouldn't work here.

Which variable is a struct?

Why do you want to do things the C way, rather than the more idiomatic
C++ forms?

fscanf requires you to get the types right, iostreams delegate the task
to the compiler.

 

[Julien]

Le 13/08/2011 12:57, Ian Collins a écrit :

On 08/13/11 09:52 PM, Julien wrote:

Please don't snip attributions, it's rude.

Sorry for this. I've got to remember this.

...

It's horrible...


Which variable is a struct?

Sorry, I made a mistake, time_t is not a struct but a datatype.

Why do you want to do things the C way, rather than the more idiomatic
C++ forms?

In fact, I try to correct cppcheck errors of a file on the internet
which is C style whereas the file has cpp extension.
I replaced the code above by this (the same way of the code you gave
before) :
time_t read_progress() {
time_t stored_secs;
std::ifstream f(CPU_TIME);
if (!f) return 0;
f >> stored_secs;
if (!f) return 0;
else return stored_secs;
}

fscanf requires you to get the types right, iostreams delegate the task
to the compiler.

Ok.
Thank you for your help. I'll do other changes to use the C++ style.

Sorry again for having snipped attributions (I never know if i cut too
little or too much)

Julien.

 

[Jorgen Grahn]

On 08/13/11 09:52 PM, Julien wrote: .... ....

Why do you want to do things the C way, rather than the more idiomatic
C++ forms?

To be fair to C, scanf() is not /the/ C way, just /a/ C way.
In both languages, I prefer to do my own parsing, using strtol() and
friends.

/Jorgen