Strange One

Programming, for all ages and all languages.
User avatar
Neo
Member
Member
Posts: 842
Joined: Wed Oct 18, 2006 9:01 am

Strange One

Post by Neo »

Can you see what is wrong with this piece of code?

Code: Select all

#include<stdio.h>
#include<string.h>
void main()
{
 volatile char another='y';
 int num;
 while (another=='y') {
  printf("enter a number");
  fflush(stdin);
  scanf("%d",&num);
  printf("\nsquare of %d is %d",num,num*num);
  printf("\nWant to enter another number y/n? ");
  fflush(stdin);
  scanf("%c",&another);
 }
}
Anyone?
Only Human
smiddy

Re:Strange One

Post by smiddy »

I'm no C expert...

The only thing that looks weird to me is the fflush(stdin);. I beleive scanf(...); will flush anything. Also you may want to check for redirection from a file (or other) too (not part of any problem I see just convention). I can't remember how to do that off the top of my head (my references are at home, I'm at work).

What are you looking for? Did you get an error and if so, what was it? What environment and compiler are you using?

[edited]

Isn't 'char another' a pointer? Then you would remove the ampersand infront of 'another' in the last scanf(...) function.

[/edited]
User avatar
Solar
Member
Member
Posts: 7615
Joined: Thu Nov 16, 2006 12:01 pm
Location: Germany
Contact:

Re:Strange One

Post by Solar »

[tt]volatile[/tt] is unnecessary. You use [tt]volatile[/tt] for stuff that could change its value outside of the program's control (e.g., hardware registers), so that the compiler doesn't cache the value.

[tt]fflush()[/tt] is unnecessary. It is defined only for output buffers.

And that's your problem: The '\n' with which you ended entering a number is still queued in stdin, since no scan function did read it from there. A crude fix would be to prepend a space to the %c:

Code: Select all

scanf( " %c", &another );
A better fix would be to [tt]getchar()[/tt] stdin until you come across the next '\n'.

Edit:

A hint for debugging: Don't assume, and print out intermediate values. I found this one by adding a [tt]printf("another: '%c'\n", another);[/tt] at the end of the loop.
Every good solution is obvious once you've found it.
Perica
Member
Member
Posts: 454
Joined: Sat Nov 25, 2006 12:50 am

Re:Strange One

Post by Perica »

..
Last edited by Perica on Tue Dec 05, 2006 9:38 pm, edited 1 time in total.
User avatar
Solar
Member
Member
Posts: 7615
Joined: Thu Nov 16, 2006 12:01 pm
Location: Germany
Contact:

Re:Strange One

Post by Solar »

Perica wrote: How can [tt]stdin[/tt] be flushed in a standards-compliant way?
You don't want to flush stdin, you want to skip everything up until the next newline. So just do that.

Code: Select all

    while (getchar() != '\n');
If you really want to flush stdin (and trust me, you don't), that'd be:

Code: Select all

    while (getchar() != EOF);
What a shitty thing the standard library is for not providing an easy way to do it.
As always, the chances are overwhelming that The Standard Ain't Broken (tm). So why don't you really want to flush stdin? Consider, for a second, the feature of input redirection, or pipes...
Every good solution is obvious once you've found it.
User avatar
Neo
Member
Member
Posts: 842
Joined: Wed Oct 18, 2006 9:01 am

Re:Strange One

Post by Neo »

Solar wrote: A crude fix would be to prepend a space to the %c:
What does the space before it do?
Only Human
User avatar
Solar
Member
Member
Posts: 7615
Joined: Thu Nov 16, 2006 12:01 pm
Location: Germany
Contact:

Re:Strange One

Post by Solar »

It stands for any amount of whitespace... thus including the linefeed.
Every good solution is obvious once you've found it.
Perica
Member
Member
Posts: 454
Joined: Sat Nov 25, 2006 12:50 am

Re:Strange One

Post by Perica »

..
Last edited by Perica on Tue Dec 05, 2006 9:41 pm, edited 1 time in total.
Perica
Member
Member
Posts: 454
Joined: Sat Nov 25, 2006 12:50 am

Re:Strange One

Post by Perica »

..
Last edited by Perica on Tue Dec 05, 2006 9:42 pm, edited 1 time in total.
Schol-R-LEA

Re:Strange One

Post by Schol-R-LEA »

Honest answer? Avoid [tt]scanf()[/tt]; use [tt]getc()[/tt] and [tt]fgets()[/tt] instead (though for all that is holy, do not use [tt]gets()[/tt], ever; an unbounded read from [tt]stdin[/tt] is just asking for a buffer overflow). While it is a serious pain, you'll get better results if you parse the input stream manually. I wouldn't normally advocate repeating functionality already available in the standard library, but in this case it is less likely to cause trouble that using [tt]scanf()[/tt] is.

In any case, you really only want to use the formatted I/O functions ([tt]printf()[/tt] and [tt]scanf()[/tt]) for, well, formatted I/O; using them to read or write a single unformatted item at a time is overkill. It's better to avoid the overhead of scanning the format string when you don't need to.

Admittedly, numeric input presents a problem, especially doubles; you have to set aside a large enough char buffer for them, read them in as a string, then convert them to the appropriate numeric format. it's a pain, and you would be wise to write a function (or two) to do it if you regularly need it:

Code: Select all

#include <math.h>
#include <stdio.h>
#include <float.h>
#include <string.h>

#define BUFFSZ 96
// this should be large enough to read in any double value

/* read in a string */ 
double getd(FILE* in)
{
    char buffer[BUFSZ]; 
    char input;
    double value;
    int i = 0;

    bool eflag, dpflag;

     eflag = dpflag = false;

    do 
    {
        input = getc(in);

        if ('\n' == input)
        {
            return (INFINITY); // I would use NAN, but it isn't standard
        }
    }  
    while (!(isdigit(in)) && '-' != input );

    do
    {
        buffer[i] = input;
        ++i;    
        input = getc(in);
 
        // check for multiple exponent markers
        if ('E' == toupper(input))
        {
               if(eflag)
               {
                   break;
               }
               else
               {
                   eflag = true;
               }
        }
        // check for multiple decimal points
        else if ('.' == input)
        {
               if(dpflag)
               {
                   break;
               }
               else
               {
                   dpflag = true;
               }
        }

    }
    while ((BUFSZ > i) && ((isdigit(input) || ('E' == toupper(input)) || ('.' == input));

    // push the last char back into the stream 
    ungetc(input, in);

    return(strtod(buffer));
}

/* eats the characters until the next newline 
    and returns the number of characters 
*/
int eatline(FILE* in)
{
    int count = 0;
  
    while ('\n' != getc(in))
    {
         ++count;
    }
    return count;
}
(Note: Not tested code)

Comments and corrections welcome. HTH.
User avatar
Pype.Clicker
Member
Member
Posts: 5964
Joined: Wed Oct 18, 2006 2:31 am
Location: In a galaxy, far, far away
Contact:

Re:Strange One

Post by Pype.Clicker »

under linux and BSD-compliant system, it looks to have "fpurge" which kills the current content of the input buffer, making room for fresh datas.

That may be interresting for special programs such as password prompters and things alike ...
User avatar
Solar
Member
Member
Posts: 7615
Joined: Thu Nov 16, 2006 12:01 pm
Location: Germany
Contact:

Re:Strange One

Post by Solar »

Hm... seems I missed this one.

I wouldn't go as far as reimplementing scanf() (which is inviting even more trouble if you ask me). All you have to do is, skip over everything in the stdin queue that you don't want to be there. If that means "everything" (and usually, it doesn't), you can just [tt]while( getchar() != EOF )[/tt].

If you expect - and want to skip - whitespaces, use that space-prepended scanf() I recommended earlier, and make sure that you use a scanf() without leading whitespace on first reading. (As this skips over either spaces, tabs, and newlines, and the one doing the input has to seperate his inputs in some way, that is usually what you want to do.)

If you want to become more sophisticated - like, skipping until the next uppercase character appears - you can use getchar() and ungetc() to "clean" stdin before doing the next scanf():
int c;
while ( ! isupper( c = getchar() ) );
ungetc( c );
Voila, the first uppercase character is still in stdin, and everything before it has been skipped. (Of course, production code must be a bit more elaborate, as you would have to check for EOF, too.)

Just don't try to reimplement scanf(). Unless you know exactly what you're doing, you'll get into even more trouble - and if you knew, we wouldn't be in this thread. ;)
Every good solution is obvious once you've found it.
mystran

Re:Strange One

Post by mystran »

I agree with Schol-R-LEA: scanf() is evil. Don't use it. It's almost as harmful as goto.
User avatar
Solar
Member
Member
Posts: 7615
Joined: Thu Nov 16, 2006 12:01 pm
Location: Germany
Contact:

Re:Strange One

Post by Solar »

mystran wrote: I agree with Schol-R-LEA: scanf() is evil. Don't use it. It's almost as harmful as goto.
Only if you use a superior, ready-made replacement. If all you do is trying to reinvent scanf() functionality, you just exchanged a tricky but proven implementation with a tricky and unproven implementation. ;-)
Every good solution is obvious once you've found it.
distantvoices
Member
Member
Posts: 1600
Joined: Wed Oct 18, 2006 11:59 am
Location: Vienna/Austria
Contact:

Re:Strange One

Post by distantvoices »

then don't implement a full fledged scanf.

only create functions which read in a number,string, what so ever, with proper boundary checking - to avoid buffer overflows.

I've done it a dozen times due to scanf not being as good to use as expected.
... the osdever formerly known as beyond infinity ...
BlueillusionOS iso image
Post Reply