-1

My question is quite similar to the one here except that I’m working with C.

I wrote some code to rotate an unsigned int; that is, the function bitRotate() (code below).

The function works very well when, instead of the printfs and scanfs, I directly put the literals I want to use, e.g. bitRotate(0xabcdef00,8);in the main function.

However, when I pass x as an argument as in the following code, abcdef00 that was got from the user, the x gets corrupted to ab000000. I checked and double checked and debugged my code multiple times and I am pretty sure the error is in this part but I don’t understand why.

#include <stdio.h>
#include <stdlib.h>
#include <limits.h>
#define WIDTH sizeof(unsigned int)*CHAR_BIT

unsigned int bitRotate(unsigned int, char );

int main()
{

    unsigned int x;
    char n;
    while(1)
    {
        printf("Enter x: ");
        scanf("%x", &x);
        printf("Enter n: ");
        scanf("%d", &n);
        printf("%x\n", bitRotate(x,n));
    }

    return 0;
}

unsigned int bitRotate(unsigned int value, char n)
{

    char un = abs(n);
    unsigned int fallen = ~(0u);

    if(un == WIDTH)
        return value;
    else if (un < WIDTH)
    {

        if (n < 0)
        {
            fallen >>= (WIDTH - n);
            fallen = value & fallen;
            fallen <<= (WIDTH - n);
            value >>= n;
        }
        else
        {
            fallen <<= (WIDTH - n);
            fallen = value & fallen;
            fallen >>= (WIDTH - n);
            value <<= n;
        }
        value |= fallen;
        return value;

    }
    else
        return 0;

}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
user401445
  • 15
  • 4
  • 2
    `"%d"` is the wrong format specifier to input a `char` via `scanf`. You need to use `"%c"`,, and to consume the newline whitespace from the previous entry, I think you need to use `" %c"` (with a leading space). [This](https://stackoverflow.com/questions/5240789/scanf-leaves-the-new-line-char-in-the-buffer) question might be helpful. – yano Oct 24 '19 at 14:00
  • At lease add `int response = getchar(); if(response == 'q') break;` to the end of your loop. – ryyker Oct 24 '19 at 14:00
  • How did you verify that you read the correct numbers? You do not check the return value of `scanf` and you don't print your numbers. How would you notice an error before you call your rotating function? – Gerhardh Oct 24 '19 at 14:02
  • 1
    Chances are good that your invalid format specifier for `scanf` leads to corruption of the number. Storing 4 or 8 bytes into a single character variable doesn't fit well. – Gerhardh Oct 24 '19 at 14:03
  • @yano I changed the type of n to int at every instance but nothing changed. I just posted here the original code without the edits I made to find the error. – user401445 Oct 24 '19 at 14:06
  • @Gerhardh i put “printf(“the number you entered was ...”). But like i said i only posted here the code without any of the debugging methods i used. – user401445 Oct 24 '19 at 14:08
  • @ryyker well, yes but I found some sources saying that this is better “for portability reasons” so I just put it like that.. Maybe on some systems a byte isn’t 8 bits? I am not sure why they recommended doing so – user401445 Oct 24 '19 at 14:09
  • @user401445 That's correct. I've never worked on a system that didn't have an 8-bit byte, but they do exist. – yano Oct 24 '19 at 14:11
  • Checking return value is not a debugging method. You should print x and n after second scanf. BTW what ist exact input for scanf? – Gerhardh Oct 24 '19 at 14:11
  • @user401445 - Yes, It had been awhile since I have seen `CHAR_BIT`, I removed the comment, and yes, it is correct to use it for what you are doing. Sorry for the confusion on that. – ryyker Oct 24 '19 at 14:14
  • 1
    @user401445 _" I changed the type of n to int at every instance but nothing changed"_: I don't believe you. – Jabberwocky Oct 24 '19 at 14:18
  • `char un = abs(n);` could be a problem. this should be: `int un = abs(n);` – ryyker Oct 24 '19 at 14:20
  • @ryyker not sure why char `un = abs(n)` could be a problem except when `n` is > 255, but for large values of `n` rotating doesn't make much sense anyway. – Jabberwocky Oct 24 '19 at 14:25
  • The code `if (n < 0)` relies on `char` being signed type which may or may not apply. You should use `signed char` instead. – Gerhardh Oct 24 '19 at 14:44

3 Answers3

0

The bitRotate function is fine and so is the way you call it.

The actual culprit is here:

char n;
scanf("%d", &n);  // <<<<<

You provide the wrong format specifier to scanf which results in undefined behaviour. Your compiler most likely warned you about this.

The %d format specifier needs an pointer to an int (which usually takes 32 bits), but you provide the pointer to a char which takes 8 bits. Therefore scanf most likely clobbers the memory adjacent to the address of n.

You want this:

int n;
scanf("%d", &n);
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • Oh well, there was one char which I didn't change.. You're right, the code worked. But I have a question, what if I don't need all of the bits of int?? I need to store an integeral value (i.e. not ASCII, if I enter 8 as the value for n at the console, I want it to be understood as 8 not as '8'), so %c won't work. What can I do instead? – user401445 Oct 24 '19 at 16:36
  • @user401445 Use `%hhd` for the format specifier. That expects the address of a `char`. – dbush Oct 24 '19 at 19:15
  • @dbush Hello, I was working on a new problem and I used the %hhd you suggested but the compiler (mingw on windows 10) gave me a warning. I googled a bit and found [this](https://stackoverflow.com/questions/10678124/mingw-gcc-unknown-conversion-type-character-h-snprintf). I tried the same idea with scanf (i.e. `_mingw_scanf()`) but it turns out that there is no function with that name. Do you have an idea how I can resolve this problem? – user401445 Nov 12 '19 at 10:36
  • @user401445 please ask another question for this. – Jabberwocky Nov 12 '19 at 10:37
0

Your code has undefined behaviour.

In this call

scanf("%d", &n);

you are using a wrong conversion specifier with an object of the type char. Moreover the type char can behave either as a signed char or as unsigned char depending on a compiler option.

Within the function there is used a wrong expression when n is negative

fallen >>= (WIDTH - n);
                 ^^^
fallen <<= (WIDTH - n);
                 ^^^

I think you mean

fallen >>= (WIDTH + n);
                 ^^^
fallen <<= (WIDTH + n);
                 ^^^

In any case the function can be written simpler. Here is a demonstrative program.

#include <stdio.h>
#include <limits.h>

unsigned int bitRotate(unsigned int value, int n )
{
    const int Width = sizeof( unsigned int ) * CHAR_BIT;

    n %= Width;

    if ( n < 0 )
    {
        value = ( value >> -n ) | ( value << ( Width + n ) );
    }
    else if ( n > 0 )
    {
        value = ( value << n ) | ( value >> ( Width - n ) );

    }

    return value;
}

int main(void) 
{
    while ( 1 )
    {
        unsigned int x;

        printf( "Enter a hexadecimal value of x (0 - exit): " );

        if ( scanf( "%x", &x ) != 1 || x == 0 ) break;

        int n;

        printf( "Enter a negative or positive value of n (0 - exit): " );
        if ( scanf( "%d", &n ) != 1 || n == 0 ) break;

        printf( "\n%#x shifted %d is %#x\n", x, n, bitRotate( x, n ) );
    }

    return 0;
}

Its output might look like

Enter a hexadecimal value of x (0 - exit): 0xabcdef00
Enter a negative or positive value of n (0 - exit): 16

0xabcdef00 shifted 16 is 0xef00abcd

Enter a hexadecimal value of x (0 - exit): 0xabcdef00
Enter a negative or positive value of n (0 - exit): -16

0xabcdef00 shifted -16 is 0xef00abcd

Enter a hexadecimal value of x (0 - exit): 0xabcdef00
Enter a negative or positive value of n (0 - exit): 32

0xabcdef00 shifted 16 is 0xabcdef00

Enter a hexadecimal value of x (0 - exit): 0
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
0

This code shows you if the value has the sign bit or not:

#include <stdio.h>
#include <stdlib.h>

int main(void) {
    const int arr[7] = {0x80000000u, -2, -1, 0, 1, 2, 0x7fffffffu};
    unsigned int i, j, signVal, uiSiz;

    uiSiz = sizeof(unsigned int);

    for (i = 0, j = 7; i < 7; i++) {
        signVal = ((unsigned int) arr[i] & (0x1u << ((8 * uiSiz) - 1))) >> ((8 * uiSiz) - 1);
        fprintf(stdout, "arr[%u]: %i, uiSiz: %u, signVal: %u;\n", i, arr[i], uiSiz, signVal);
    }

    return 0;
}

LINK @[ https://ideone.com/rWiSrF ]

C. R. Ward
  • 93
  • 5