«

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.

On 08/13/11 09:12 PM, Julien wrote:
> 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;

--
Ian Collins

>> ...
>
> 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

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

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

> I wrote:

>> 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 ?

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.

--
Ian Collins

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.
> ...
>>> ...
>> 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?
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.

On Sat, 2011-08-13, Ian Collins wrote:
> On 08/13/11 09:52 PM, Julien wrote:
....
>> int n = fscanf(f, "%ld",&stored_secs);
....

> 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

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .