fscanf hangs

P

PeterOut

If you keep posting to irrelevant groups, no one will want to help
you.

The problem is that I am not sure if it a problem with my C code or a
problem with MS Visual C++.
 
B

Barry Schwarz

The problem is that I am not sure if it a problem with my C code or a
problem with MS Visual C++.

Now you are just being disingenuous. In spite of the product name,
your code was C, not C++. The product contains both compilers. But
you already new that since you were compiling C code. And you
certainly didn't invoke any tools. That eliminates two of the
extraneous newsgroups. And since you already know there is a
collection of Microsoft newsgroups, you could have found the one that
deals with the Microsoft version of the language pretty easily.


Remove del for email
 
P

PeterOut

Hmmm, I was thinking that it might be a consequence of corrupt memory
(like buffer overruns, changing memory after free() (or equivalent) or
some other similar problem).
Test if you have access to a function called AfxCheckMemory(). (It is
available in later versions of their compiler, don't know if it is in
that version however).
If it is available try placing

ASSERT(AfxCheckMemory());

immediately before the call to fscanf(). If it breaks you have
previously overwritten some memory somewhere in your program where that
should not happen...- Hide quoted text -

- Show quoted text -

Hi Johan,

That looks likfe a handy function. Thanks for bringing it to my
attention. I put it in and it didn't flag anything. I have decided
to give up on fscanf and add the following code instead.

ERROR_NUMBER FReadLine(FILE *fpInputFile, char *csBuffer)
{
int i;

if ((csBuffer[0] = fgetc( fpInputFile )) == EOF) return
ERROR_READING_FILE;
i = (csBuffer[0] == '\n')? 0 : 1;
while ((csBuffer = fgetc( fpInputFile )) != '\n')
{
if (csBuffer[i++] == EOF) return ERROR_READING_FILE;
}
csBuffer = '\0';

return ERROR_NONE;
}

if ((fpFile = fopen(csFileName, "r")) == NULL)
{
// Error handling
}
{
float fMean;
long lX, lY, lLastX = fppPlane->iColumns-1, lLastY = fppPlane-
char csBuffer[32];

do
{
if ((enErrorNumber=FReadLine(fpFile, csBuffer))!=ERROR_NONE) break;
lX=atoi(strtok(csBuffer, "\t"));
lY=atoi(strtok(NULL, "\t"));
fMean=(float)atof(strtok(NULL, "\t"));
} while (lX < lLastX || lY < lLastY);
}
fclose(fpFile);

So far no problem.

Thanks again for your help,
Peter.
 
B

Barry Schwarz

snip obsolete code that should not have been quoted
Hi Johan,

That looks likfe a handy function. Thanks for bringing it to my
attention. I put it in and it didn't flag anything. I have decided
to give up on fscanf and add the following code instead.

ERROR_NUMBER FReadLine(FILE *fpInputFile, char *csBuffer)
{
int i;

if ((csBuffer[0] = fgetc( fpInputFile )) == EOF) return
ERROR_READING_FILE;
i = (csBuffer[0] == '\n')? 0 : 1;
while ((csBuffer = fgetc( fpInputFile )) != '\n')
{
if (csBuffer[i++] == EOF) return ERROR_READING_FILE;
}
csBuffer = '\0';

return ERROR_NONE;
}


This sure seems like a lot of work just avoid calling fgets.
if ((fpFile = fopen(csFileName, "r")) == NULL)
{
// Error handling
}
{
float fMean;
long lX, lY, lLastX = fppPlane->iColumns-1, lLastY = fppPlane-
char csBuffer[32];

Is 32 bytes really sufficient to hold the longest line in your file?
do
{
if ((enErrorNumber=FReadLine(fpFile, csBuffer))!=ERROR_NONE) break;
lX=atoi(strtok(csBuffer, "\t"));
lY=atoi(strtok(NULL, "\t"));
fMean=(float)atof(strtok(NULL, "\t"));

The cast is superfluous.
} while (lX < lLastX || lY < lLastY);
}
fclose(fpFile);

So far no problem.

So you have managed to camouflage the problem with no understanding of
what the real issue was?
Thanks again for your help,
Peter.


Remove del for email
 
M

Mark McIntyre


Precisely.
--
Mark McIntyre

"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it."
--Brian Kernighan
 
P

PeterOut

This sure seems like a lot of work just avoid calling fgets.
MUCH more than worth it. I have replaced a black box that hangs with
much more transparent code that does not. If it did hang or crash, I
would have a much better idea of why.
Is 32 bytes really sufficient to hold the longest line in your file?
Excellent point. I will modify FReadLine() so that there is a maximum
to the number of characters that are read.
The cast is superfluous.

fMean=atof(strtok(NULL, "\t"));
warning C4244: '=' : conversion from 'double' to 'float', possible
loss of data
So you have managed to camouflage the problem with no understanding of
what the real issue was?

I may have evaded the issue but have replaced a black box, that hangs
with no error message, with more transparent code. fscanf probably
hung because it was waiting for a particular input. It was not
suitable for this application since the expected input may not have
been in the input file. I think my modification has given me more
control and has made the code potentially more robust.

Thanks,
Peter.
 
R

Richard Heathfield

PeterOut said:
fMean=atof(strtok(NULL, "\t"));
warning C4244: '=' : conversion from 'double' to 'float', possible
loss of data

The diagnostic message, whilst not compulsory, is useful, in that it
tells you that the conversion may not be able to retain all the
precision that would be available to you were you to use double.

Suppressing the diagnostic information is like taking the battery out of
your doorbell. There's still someone at the door, even if you can't
hear them any more.

Frankly, I'd use double. (And while I was at it, I'd use strtod instead
of atof and strtol instead of atoi.)

I certainly wouldn't pass strtok(NULL, "\t") to anything expecting a
string as an argument, in case the string you're parsing turned out to
be poorly formed.

<snip>
 
C

Chris Torek

MUCH more than worth it. I have replaced a black box that hangs with
much more transparent code that does not. ...

I think Barry Schwarz's point was that you could just call fgets()
(*not* fscanf()!), instead of using another 15 or so lines of code
to re-implement fgets().
I may have evaded the issue but have replaced a black box, that hangs
with no error message, with more transparent code. fscanf probably
hung because it was waiting for a particular input. It was not
suitable for this application since the expected input may not have
been in the input file.

Actually, fscanf() probably did *not* hang. Instead, since you
had code of the general form [%]:

some_sort_of_loop {
/* ignore result of */ fscanf(file, format, args);
if (feof(file)) break;
... do work ...
}

what would happen is that fscanf() would encounter "bad" input
and *leave the input in the stream*. The feof() test would say
"not EOF" -- fscanf() failed, yes, but not because of EOF -- and
on the next iteration, the fscanf() would again immediately
fail.

Using fgets() (instead of fscanf()) *is* a good idea. In general,
of the scanf family, the sscanf() function is the most useful /
least error-prone -- but even then there are pitfalls. Start with
fgets() (or equivalent), then break up the resulting lines as
desired, using sscanf() or the strto* functions, or whatever.

[% Any time anyone ignores the return value of an input-reading
function, whether that is one of the scanf family, or fgets(), or
getchar(), or whatever it may be, this is a nearly-certain sign
that the code is wrong. But at least the loop above avoids the
even more common beginner's error, which is to call feof() *before*
calling an input-reading function. Since feof() only tells you
*why* a previous read failed -- it has to be one that already failed
-- calling it *before* observing such a failure is just asking for
trouble. Calling it after observing failure is still somewhat
suspect, since many people seem not to realize that Standard C
provides for exactly two separate kinds of failure: "failure due
to ordinary end of file", for which feof() will say "yes" and
ferror() will say "no", and "failure due to bad floppy disk, or
cpu catching fire, or any other not-ordinary-EOF-disaster", for
which feof() will say "no" and ferror() will say "yes".]
 
R

Richard Heathfield

Chris Torek said:

[...] many people seem not to realize that Standard C
provides for exactly two separate kinds of failure: "failure due
to ordinary end of file", for which feof() will say "yes" and
ferror() will say "no",
....sure.

and "failure due to bad floppy disk, or
cpu catching fire, or any other not-ordinary-EOF-disaster", for
which feof() will say "no" and ferror() will say "yes".]

Okay, okay, I hear you, but wait a minute... As the chicken says:

Ginger: Listen. We'll either die free chickens or we die trying.
Babs: Are those the only choices?

That is, does the Standard guarantee that (feof(fp) && ferror(fp))
evaluates to 0? Is there not scope for some condition in which, say, 32
bytes were requested, there were only 30 bytes left on the file, so EOF
would certainly be encountered, and only 16 bytes could be read because
the next bit of the file was in a bad sector, so that both Bad Things
are happening at once?
 
C

Chris Torek

... That is, does the Standard guarantee that (feof(fp) && ferror(fp))
evaluates to 0?

Definitely not. In particular, since the error indicators are
"sticky" -- that is, they require use of clearerr(fp) to clear them
-- we could certainly have, for instance:

int c1, c2;
c1 = getc(stdin);
c2 = getc(stdin);

set both c1 and c2 to EOF, with the first EOF being due to the user
pressing the system's "EOF key" (^D or ^Z or whatever) and the
second being due to the user logging off ("terminating the session",
or whatever the system calls it). The first might set feof(fp),
and the second set ferror(fp).

I meant after a single error, but even then I am not sure:
Is there not scope for some condition in which, say, 32
bytes were requested, there were only 30 bytes left on the file, so EOF
would certainly be encountered, and only 16 bytes could be read because
the next bit of the file was in a bad sector, so that both Bad Things
are happening at once?

That seems plausible.
 
P

PeterOut

PeterOut said:



The diagnostic message, whilst not compulsory, is useful, in that it
tells you that the conversion may not be able to retain all the
precision that would be available to you were you to use double.

Suppressing the diagnostic information is like taking the battery out of
your doorbell. There's still someone at the door, even if you can't
hear them any more.

Frankly, I'd use double. (And while I was at it, I'd use strtod instead
of atof and strtol instead of atoi.)

I've found that doubles tend to increase the processing time relative
to singles and the data I work with (biological) does not have that
level of accuracy.
I certainly wouldn't pass strtok(NULL, "\t") to anything expecting a
string as an argument, in case the string you're parsing turned out to
be poorly formed.

I have been looking into using strchr instead but it appears that my
problem is a deadlock. Multithreading was the default for the
development platform I am using (MS Visual C++ 6.0) but I am not
really an expert in multithreaded programming Maybe I should stick to
single threaded until I know what I want to do with multithreaded.

Thanks,
Peter.
 
P

PeterOut

I think Barry Schwarz's point was that you could just call fgets()
(*not* fscanf()!), instead of using another 15 or so lines of code
to re-implement fgets().

Yes, I can see your point. I essentially re-implemented fgets() after
I added the maximum number of bytes to read. I will use fgets() since
it has probably been tested and optimized a lot more thoroughly than
my re-implemtation. I will also tryi using sscanf instead of
strtok(). However, it is beginning to appear that my problem is a
deadlock which tends to arise in multihtreaded programming.

Thanks,
Peter.
 
B

Ben Pfaff

Richard Heathfield said:
Suppressing the diagnostic information is like taking the battery out of
your doorbell.

Doorbells are battery-powered in England? Yet another way that
it's such a foreign country...
 
C

CBFalconer

Ben said:
Doorbells are battery-powered in England? Yet another way that
it's such a foreign country...

Some are in the USA also. A battery is considerably cheaper than a
transformer, and works even when the power is out! This is
magnified when the mains are 250 VAC. Of course, with the usual
lack of maintenance, the doorbells don't work.
 
F

Flash Gordon

Ben Pfaff wrote, On 11/07/07 03:28:
Doorbells are battery-powered in England?

Some of them are, some are not.
> Yet another way that
it's such a foreign country...

No, yours is the foreign country, what's worse it is full of foreigners ;-)
 
F

Flash Gordon

PeterOut wrote, On 11/07/07 02:11:
I've found that doubles tend to increase the processing time relative
to singles and the data I work with (biological) does not have that
level of accuracy.

If you have measured and found that for your work this is the case, then
it is a valid reason for using float rather than double. Just remember
that this is not the case for all hardware or all programs on some
specific pieces of hardware. Also remember that double normally supports
a larger range than float, not just greater precision.

The comment about strtol instead of atoi is still valid, because that is
about the ability to detect and handle input errors.
I have been looking into using strchr instead but it appears that my
problem is a deadlock. Multithreading was the default for the
development platform I am using (MS Visual C++ 6.0) but I am not
really an expert in multithreaded programming Maybe I should stick to
single threaded until I know what I want to do with multithreaded.

If using multithreading, or you might go back to it in the future, then
that is an even bigger reason not to use strtok. However, yes,
multithreading does give you a number of additional headaches such as
deadlock if you are not experienced (or even if you are) so you should
only use threads if you have a real reason to do so.

Threads are off topic here. There is I believe a group specifically for
threads, and there are lots of platform specific groups.
 
R

Richard Heathfield

Ben Pfaff said:
Doorbells are battery-powered in England?

Well, some are, yes. And some aren't. And some, like mine, or more sort
of kinda. My setup is as follows:

On the door, a box for the pushbutton. It contains a battery and a radio
transmitter.

In the kitchen, a box plugged into the mains. It contains a radio
receiver and the actual speaker.

It's a completely wireless system. Even the sound box has no power cable
- the three pins are right there on the box, so you just stick it in
the wall.

The only minor disadvantage is that, when you first install it, you have
to fiddle around with the channel settings a bit to avoid clashing with
other similar radio devices in the area. It can take a few days to work
out the best channel. Apart from that, though, it's great.

Yet another way that it's such a foreign country...

England isn't foreign. You must be thinking of somewhere else.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,780
Messages
2,569,608
Members
45,250
Latest member
Charlesreero

Latest Threads

Top