Simple C help

AARRRGH

Hello

I was wondering if someone could help me with this.

It's been years since I used C. For some reason, in these three lines of code -

// allow the user to change the bit
printf("Change the bit value to 1 or 0: ");
fgets(changeBit, sizeof(changeBit), stdin);
sscanf(changeBit, "%d", &changeDecimal);

if (changeDecimal == 1)

- the user is not getting prompted to enter a number. It's just skipping past the fgets and going straight to the if statement.

Can anyone see any reason for this??

Below is my full code. I'm basically just doing a bit of bit manipulation.

Thanks!

#include <stdio.h>

// the string
unsigned char line[11];

// the nth byte in the string
char firstNum[3];

// the nth byte in the string converted to decimal
int firstNumDecimal;

// the bit we want to use
char secondNum[2];

// the bit we want to use converted to decimal 
int secondNumDecimal;

// the bit to change
char changeBit[2];

// the bit to change converted to decimal
int changeDecimal = 0;

// the bitmask
unsigned char bitMask = 0x80;

// the char in binary
unsigned char charInBinary = 0x00;


// just a counter...
int count = 0;

main()
{
        // read in the string
        printf("Enter a string, 10 chars max: ");
        fgets(line, sizeof(line), stdin);

        // read in the nth byte in the string
        // between 0 and strlen(line)-2
        printf("Enter a number between 0 and %d: ", strlen(line)-2);
        fgets(firstNum, sizeof(firstNum), stdin); 
        sscanf(firstNum, "%d", &firstNumDecimal);

        // read in the bit we want to use
        printf("Enter a number between 1 and 8: ");
        fgets(secondNum, sizeof(secondNum), stdin);
        sscanf(secondNum, "%d", &secondNumDecimal);

        // print out the char in various formats
        printf("That char is %xh in hex and %d in decimal\n", line[firstNumDecimal], line[firstNumDecimal]);

	// print out the binary for the char
        printf("That char in binary format is ");

        for(count = 0; count < 8; count++) {

                charInBinary = line[firstNumDecimal] & bitMask;

                if (charInBinary==0)
                        printf("0");
                else
                        printf("1");

                bitMask >>= 1;
        }

        printf("\n");

	// print out the bit the user would like to see
	// reset the bitmask
        bitMask = 0x01;

	charInBinary = line[firstNumDecimal] & (bitMask << secondNumDecimal);

	if (charInBinary==0)
        	printf("The value of the bit is: 0\n");
        else
                printf("The value of the bit is: 1\n");
	
	// allow the user to change the bit
	printf("Change the bit value to 1 or 0: ");
        fgets(changeBit, sizeof(changeBit), stdin);
        sscanf(changeBit, "%d", &changeDecimal);

	if (changeDecimal == 1)
		line[firstNumDecimal]|= (bitMask << secondNumDecimal);
	else
		line[firstNumDecimal]&= ~(bitMask << secondNumDecimal);

        // print out the char in various formats
        printf("That char is %xh in hex and %d in decimal\n", line[firstNumDecimal], line[firstNumDecimal]);

	// print out the binary for the char
	// reset the bitmask
	bitMask = 0x08;

        printf("That char in binary format is ");

        for(count = 0; count < 8; count++) {

                charInBinary = line[firstNumDecimal] & bitMask;

                if (charInBinary==0)
                        printf("0");
                else
                        printf("1");

                bitMask >>= 1;
        }

        printf("\n");

}

Tobias Greeshman

try a fflush(stdin)

The stdin stream was probably not cleared by the OS in time for your call to fgets, as you done some IO before this call. So the stream could contain something ending in a newline and accepts that as input.

AARRRGH

Thanks silas. No change unfortunately...

ianhobo

What are you using to compile it?

adding fflush as was suggested worked for me.
Without it, the program skips that line, with it in, the user is asked for the number

printf("Change the bit value to 1 or 0: ");
fflush(stdin);
fgets(changeBit, sizeof(changeBit), stdin);
sscanf(changeBit, "%d", &changeDecimal);

AARRRGH

Hi ianhobo

I'm using GCC on Linux.

It's a weird one! Googling isn't helping much unfortunately...

Tobias Greeshman

try getchar(), use it instead of the call to fgets().

ianhobo

sorry, I assumed you were on VStudio on windows
There was a similar problem recently, where getChar() fixed it, but the circumstances are different this time. might be worth trying though.

I build it in visual studio 9, ill try gcc on windows now and let you know

lynchie

dublindude wrote: »

Hello

I was wondering if someone could help me with this.

It's been years since I used C. For some reason, in these three lines of code -

// allow the user to change the bit
printf("Change the bit value to 1 or 0: ");
fgets(changeBit, sizeof(changeBit), stdin);
sscanf(changeBit, "%d", &changeDecimal);

if (changeDecimal == 1)

- the user is not getting prompted to enter a number. It's just skipping past the fgets and going straight to the if statement.

Can anyone see any reason for this??

Below is my full code. I'm basically just doing a bit of bit manipulation.

Thanks!

#include <stdio.h>

// the string
[b]
unsigned char line[11];
[/b]
// the nth byte in the string
char firstNum[3];

// the nth byte in the string converted to decimal
int firstNumDecimal;

// the bit we want to use
[b]char secondNum[2];
[/b]
// the bit we want to use converted to decimal 
int secondNumDecimal;

// the bit to change
char changeBit[2];

// the bit to change converted to decimal
int changeDecimal = 0;

// the bitmask
unsigned char bitMask = 0x80;

// the char in binary
unsigned char charInBinary = 0x00;


// just a counter...
int count = 0;

main()
{
        // read in the string
        printf("Enter a string, 10 chars max: ");
        fgets(line, sizeof(line), stdin);

        // read in the nth byte in the string
        // between 0 and strlen(line)-2
        printf("Enter a number between 0 and %d: ", strlen(line)-2);
        fgets(firstNum, sizeof(firstNum), stdin); 
        sscanf(firstNum, "%d", &firstNumDecimal);

        // read in the bit we want to use
        printf("Enter a number between 1 and 8: ");
        fgets(secondNum, sizeof(secondNum), stdin);
        sscanf(secondNum, "%d", &secondNumDecimal);

        // print out the char in various formats
        printf("That char is %xh in hex and %d in decimal\n", line[firstNumDecimal], line[firstNumDecimal]);

	// print out the binary for the char
        printf("That char in binary format is ");

        for(count = 0; count < 8; count++) {

                charInBinary = line[firstNumDecimal] & bitMask;

                if (charInBinary==0)
                        printf("0");
                else
                        printf("1");

                bitMask >>= 1;
        }

        printf("\n");

	// print out the bit the user would like to see
	// reset the bitmask
        bitMask = 0x01;

	charInBinary = line[firstNumDecimal] & (bitMask << secondNumDecimal);

	if (charInBinary==0)
        	printf("The value of the bit is: 0\n");
        else
                printf("The value of the bit is: 1\n");
	
	// allow the user to change the bit
	printf("Change the bit value to 1 or 0: ");
        fgets(changeBit, sizeof(changeBit), stdin);
        sscanf(changeBit, "%d", &changeDecimal);

	if (changeDecimal == 1)
		line[firstNumDecimal]|= (bitMask << secondNumDecimal);
	else
		line[firstNumDecimal]&= ~(bitMask << secondNumDecimal);

        // print out the char in various formats
        printf("That char is %xh in hex and %d in decimal\n", line[firstNumDecimal], line[firstNumDecimal]);

	// print out the binary for the char
	// reset the bitmask
	bitMask = 0x08;

        printf("That char in binary format is ");

        for(count = 0; count < 8; count++) {

                charInBinary = line[firstNumDecimal] & bitMask;

                if (charInBinary==0)
                        printf("0");
                else
                        printf("1");

                bitMask >>= 1;
        }

        printf("\n");

}

The two fields above in bold are the wrong size. You need to keep sapce for max chars +2 as fgets appends \0 \n to the input string

carveone

First of all, fflush(stdin) does nothing. At least on Unix systems. But that's beside the point - you are heading up the wrong alley there...

Be careful - you are reading from a "cooked mode" (as opposed to raw where you get a character immediately) input stream.

Without doing too much analysis of your code, I feel this is what is happening:

You prompt for an input -
printf("Enter a number between 1 and 8: ");

You type in "3" and hit return.
The input stream now contains: "3\n".

fgets(secondNum, sizeof(secondNum), stdin);

That reads a single character from the buffered input stream - the '3'. The buffer now contains "\n".

The next read using fgets will read .... What?... the "\n".

printf("Change the bit value to 1 or 0: ");
fgets(changeBit, sizeof(changeBit), stdin);

You are asking to read a single character here. 10 to 1 it contains a "\n". Print out changeBit[0] in hex and see does it contain 0x10.

Conor.

ianhobo

carveone wrote: »

That reads a single character from the buffered input stream - the '3'. The buffer now contains "\n".

The next read using fgets will read .... What?... the "\n".

I dont think so, if you check the actual sizeof(secondnum), its two, so what your saying is not correct. both the number and the \n will be read from the stream. see pic



lynchie: as for \0\n
fgets() needs space for N elements plus 1... N+1, not N+2 for the \n.
http://en.wikipedia.org/wiki/Fgets

carveone

ianhobo wrote: »

I dont think so, if you check the actual sizeof(secondnum), its two, so what your saying is not correct. both the number and the \n will be read from the stream. see pic

It's two so fgets will read the number and then terminate the string with '\0'. If it read the \n too the string would be unterminated. See below where I paste from the Borland C help file... fgets reads at most sizeof(secondnum) - 1 chars and then terminates the string.

Try my test (printing the results of the skipped fgets and see what you get.

lynchie wrote:

The two fields above in bold are the wrong size. You need to keep sapce for max chars +2 as fgets appends \r\n to the input string

You're right but for the wrong reason

From the borlandC help:

char *fgets(char *s, int n, FILE *stream);

fgets reads characters from stream into the string s. The function stops reading when it reads either n - 1 characters or a newline character whichever comes first. fgets retains the newline character at the end of s. A null byte is appended to s to mark the end of the string.

So fgets just makes sure the string is null terminated.

Do this:

char buffer[80];

...

if (fgets(buffer, sizeof(buffer), stdin) == NULL)
{
    //error - eof or input error
    ....
}

i = strlen(buffer);

if (i == 0 || (buffer[i - 1] != '\n'))
{
    // error - buffer too short/too long
    // (.. although i == 0 means that fgets returned an eof...)
}

buffer[i - 1] = '\0';   // chomp out the \n

sscanf( ... etc)

Conor.

lynchie

carveone wrote: »

It's two so fgets will read the number and then terminate the string with '\0'. If it read the \n too the string would be unterminated. See below where I paste from the Borland C help file... fgets reads at most sizeof(secondnum) - 1 chars and then terminates the string.

Try my test (printing the results of the skipped fgets and see what you get.



You're right but for the wrong reason

From the borlandC help:

char *fgets(char *s, int n, FILE *stream);

fgets reads characters from stream into the string s. The function stops reading when it reads either n - 1 characters or a newline character whichever comes first. fgets retains the newline character at the end of s. A null byte is appended to s to mark the end of the string.

Conor.

I had edited my post after I re-read it as I had meant to say \0 instead!

ianhobo

Some similar problems here, it seems fgets is the problem
Seen as all of the arrays are of known sizes, could you use scanfs?

http://www.trap17.com/index.php/error-using-fgets-strcmp_t33750.html
http://www.daniweb.com/forums/thread57099.html

and this: from here

This is because fgets() stops reading a line when it finds a newline character (a line-feed), so it didn't see the carriage-return until it started reading the next line.

The function stops reading when it reads either n - 1 characters or a newline character whichever comes first.

ok, well I thing contradicts with my wikipedia explanation: (sorry, i couldn't find a "better" one)

The fgets function terminates reading after a new-line character is found.

?

But the statement after that in the article sort of eludes to what your saying?

The length argument includes space needed for the null character which will be appended to the end of the string.

Accroding to that sizeof(secondNUm) which is 2 should be enough, should it not?

Many internet searches later yields only one conclusive answer....don't use fgets()

Tobias Greeshman

ianhobo wrote: »

Many internet searches later yields only one conclusive answer....don't use fgets()

A bit extreme surely, to have that attitude and not use a a function out of the c standard library because it has some side-effects (well not really side effects, but what some might call unexpected behavour).

Looking at the man page for fgets, it does state about the up to length-1, and the newline being a delimiter inside the input too, although it could be stated a little clearer.

Certainly don't be afraid to use fgets, just be aware of how to use it correctly to get the desired result.

carveone

ianhobo wrote: »

Some similar problems here, it seems fgets is the problem

I wouldn't think so. It's a library function, doing what you told it to...

http://www.trap17.com/index.php/error-using-fgets-strcmp_t33750.html

This first reference repeats what I've said and the same solution too. Sigh, I wish turbo-c was still around. I liked the tremendously easy way you could single step through code and watch variables - stuff like this becomes really obvious then.

and this: from here

Goes on about cr-lf translation issues. Meh. Thought you were using gcc or something so it wouldn't matter. You seem very loath to go and do some debugging yourself. Go printf the contents of what you are reading in:

fgets(changeBit, sizeof(changeBit), stdin);
printf("changeBit is: %02X and %02X\n", changeBit[0], changeBit[1]);

Accroding to that sizeof(secondNUm) which is 2 should be enough, should it not?

Enough for what? One character? Sure. If that's what you want. I think you are misunderstanding: The input is buffered - there's all these characters sitting in the buffer waiting to be read.

What happens if you type: 2000<return>

Then repetitive calling of fgets 5 times, with the size parameter of 2 into the area pointed to by secondNum will get:

2
0
0
0
\n

Don't see how it is supposed to do anything else given that there is only enough space for 1 character!

If you call fgets with (buffer, 80, stdin) you will get "2000\n". If you call it with (buffer, 80000, stdin) you will get "2000\n". If you call it with (buffer, 3, stdin) you will get "20" (3 bytes - '2', '0', '\0'). OK? Whew!

In the last case, there will still be "00\n" sitting in the input buffer. So if you next call fgets(buffer, 80000, stdin), you will get "00\n". Not sure how I can be any clearer

silas wrote:

Looking at the man page for fgets, it does state about the up to length-1, and the newline being a delimiter inside the input too, although it could be stated a little clearer.

Yes. But I don't see how it could be clearer! fgets very purpose is to read line oriented input, with 1) a constriction on running off the end of the buffer, and 2) making sure the returned string is valid (ie: terminated). Oh and 3) gets is an abhorrence.

Conor (argh!).

AARRRGH

Thanks everyone for your help!

It's actually working now. I increased the array sizes to include room for the \n and it seems to have fixed it.

Here's my final working code if anyone's interested in bit manipulation...

#include <stdio.h>

unsigned char line[12]; // the string

char byteAsChar[4]; // the nth byte in the string
int byteAsDecimal; // the nth byte in the string converted to decimal

char bitAsChar[3]; // the bit we want to use
int bitAsDecimal; // the bit we want to use converted to decimal 

char changeBitAsChar[3]; // the bit to change
int changeBitAsDecimal; // the bit to change converted to decimal

unsigned char bitMask = 0x80; // the bitmask

unsigned char charInBinary = 0x00; // the char in binary

int count = 0; // just a counter...

main()
{
        // read in the string
        printf("Enter a string, 10 chars max: ");
        fgets(line, sizeof(line), stdin);

        // read in the nth byte in the string
        // between 0 and strlen(line)-2
        printf("Enter the byte number (0 - &#37;d): ", strlen(line)-2);
        fgets(byteAsChar, sizeof(byteAsChar), stdin); 
        sscanf(byteAsChar, "%d", &byteAsDecimal);

        // read in the bit we want to use
        // 1 is the least significant bit
        printf("Enter the bit number (1 - 8): ");
        fgets(bitAsChar, sizeof(bitAsChar), stdin);
        sscanf(bitAsChar, "%d", &bitAsDecimal);

        // when we are shifting bits, we want it to be one less than the bit the users wants to manipulate
        // for example, if he was interested in the third bit he would type 3, but we only need to
        // shift two places to the left from 0x01
        // e.g. 0000 0001
        // moved one position to the left, 0000 0010
        // moved two positions to the left, 0000 0100
        // so we only had to move two positions to get the third bit
        bitAsDecimal-=1;

        // print out the char in various formats
        printf("That byte is %xh in hex and %d in decimal\n", line[byteAsDecimal], line[byteAsDecimal]);

        // print out the binary for the char
        printf("That byte in binary format is ");

        // so the logic here is
        // & the eight bit in the first byte with the eigth bit in the bitmask. An & needs both bits to be
        // 1 for a 1 result, so if the result is 0, we know the eight bit in the byte is a 0
        // repeat this for all bits, shifting the 1 in the bitmask all the way to the least significant bit

        for(count = 0; count < 8; count++) {
                charInBinary = line[byteAsDecimal] & bitMask;

                if (charInBinary==0)
                        printf("0");
                else
                        printf("1");

                bitMask >>= 1;
        }

        printf("\n");

        // print out the bit the user would like to see
        // reset the bitmask
        bitMask = 0x01;

        charInBinary = line[byteAsDecimal] & (bitMask << bitAsDecimal);

        if (charInBinary==0)
                printf("The value of the bit is: 0\n");
        else
                printf("The value of the bit is: 1\n");


        // allow the user to change the bit
        printf("Change the bit value to 1 or 0: ");
        fgets(changeBitAsChar, sizeof(changeBitAsChar), stdin);
        sscanf(changeBitAsChar, "%d", &changeBitAsDecimal);
        
        // for some unknown reason, the fgets above is sometimes being skipped
        // if this is happening to you, please just hardcode the bit to 1 or 0

        // reset the bitmask
        bitMask = 0x01;

        if (changeBitAsDecimal == 1) {

        // so the logic here is
        // | the bit in the first byte with the same bit in the bitmask. An | only needs one of the bits to be
        // 1 for a 1 result, so by ORing the bit with a 1, we are guaranteed the bit will become a 1
        // the result is written back to the byte

                printf("Changing that bit to a 1...\n");
                line[byteAsDecimal]|= (bitMask << bitAsDecimal);
        }
        else {

        // so the logic here is
        // & the bit in the first byte with the same bit in the bitmask. An & needs both bits to be
        // 1 for a 1 result, so by NOTing the bitmask to make it a 0, we are guaranteed the bit will become a 0
        // the result is written back to the byte

                printf("Changing that bit to a 0...\n");
                line[byteAsDecimal]&= ~(bitMask << bitAsDecimal);
        }


        // print out the char in various formats
        printf("That byte is %xh in hex and %d in decimal\n", line[byteAsDecimal], line[byteAsDecimal]);

        // print out the binary for the char
        // reset the bitmask
        bitMask = 0x80;

        printf("The byte in binary format is now ");

        for(count = 0; count < 8; count++) {

                charInBinary = line[byteAsDecimal] & bitMask;

                if (charInBinary==0)
                        printf("0");
                else
                        printf("1");

                bitMask >>= 1;
        }

        printf("\n");
}

ianhobo

You seem very loath to go and do some debugging yourself. Go printf the contents of what you are reading in

In fairness, I was the first to post some debugging! With a nice screen shot attachment too. I guess I just don't have as much free time in work as others to just go off debugging random code...

You said:

Without doing too much analysis of your code

See above

I wouldn't think so. It's a library function, doing what you told it to...

C Libraries are far from infallible - strcpy() enyone?

I already posted how my reply was linked to the explanation that I had found, one which makes no mention of N-1.
I was wrong, I'm not too worried about it either because now I understand it all a bit better

carveone

ianhobo wrote: »

In fairness, I was the first to post some debugging! With a nice screen shot attachment too. I guess I just don't have as much free time in work as others to just go off debugging random code...

Ooops! Apologies ianhobo, at some point I confused you with the original poster, which is why I said that. There was no offence meant

Conor.

ianhobo

No worries

AARRRGH

It's actually been years since I used C, so I forgot what a niggly language it is. PHP makes you a lazy programmer I suppose! The idea of a \n being stuck in a buffer is a way of thinking I'd long forgotten about.

Thanks for the help everyone.