Circular right and left shift in C

Question

I'm doing circular right shift and left shift in C, I'm wrong somewhere. For right rotation if I give the input as 123 and number of rotations as 3, the output what I get is wrong. Help me to find the mistake please.

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

void rotateLeft(unsigned long int num,int n);
void rotateRight(unsigned long int num,int n);
void bin_print(unsigned long int num);


int main()
{
    printf("\tThis program is to circular right & left shift the int number by n\n\n");
    printf("Possible operations\n1. circular right shift\n2. circular left shift\n");
    int choice,n;
    unsigned long int num;
    printf("Enter your choice: ");
    scanf("%d",&choice);
    printf("Enter a number: ");
    scanf("%lu", &num);
    bin_print(num);
    printf("Enter number of rotation: ");
    scanf("%d", &n);
    (choice==1) ? rotateRight(num,n) : rotateLeft(num,n);
}

void bin_print(unsigned long int num)
{
    for(int i = 31; i >= 0; i--)
    {
        if((num & (1 << i))) {
            printf("%d",1);         // The ith digit is one
        }
        else {
            printf("%d",0);         // The ith digit is zero
        }
        if(i%8==0) printf(" ");
    }
    printf("\n");
}

void rotateLeft(unsigned long int num, int n)
{
    unsigned long int val = (num << n) | (num >> (32 - n));
    bin_print(val);
    printf("%ld",val);
}

void rotateRight(unsigned long int num,int n)
{
    unsigned long int val = (num >> n) | (num << (32 - n));
    bin_print(val);
    printf("%ld",val);
}

`(1 << i)` is wrong and undefined behavior. The constant `1` is of type `int` and when you left shift something 31 bits into such a type, the behavior is undefined. You should be using `1u`. — Lundin, Jan 05 '23 at 11:56
@BalajiBaski "the output what I get is wrong" --> what was the output? What did you expect? — chux - Reinstate Monica, Jan 05 '23 at 12:35
You also shouldn't take it for granted that `int`/`unsigned`/`long`/`unsigned long` are 32 bits. — Ted Lyngmo, Jan 05 '23 at 12:35
`unsigned long int` is 64 bit on my system so I can't use your code — Support Ukraine, Jan 05 '23 at 12:38

chux - Reinstate Monica · Accepted Answer · 2023-01-05T13:39:32.993

Do not assume the width

Code assumes unsigned long is 32-bit. Its width must be at least 32-bit, but could be more, like 64.

int constant

1 << i is a shifted int, yet code needs a shifted unsigned long. Use 1UL << i.

Use a matching print specifier @Support Ukraine

This implies OP might not have enabled all warnings. Save time. Enable all compiler warnings.

// printf("%ld",val);
printf("%lu",val);

#include <limits.h>

#if ULONG_MAX == 0xFFFFFFFFu
  #define ULONG_WIDTH 32
#elif ULONG_MAX == 0xFFFFFFFFFFFFFFFFu
  #define ULONG_WIDTH 64
#else
  #error TBD code
#endif

void bin_print(unsigned long int num) {
    // for(int i = 31; i >= 0; i--)
    for(int i = ULONG_WIDTH - 1; i >= 0; i--)
    ...

        // if((num & (1 << i))) {
        if((num & (1UL << i))) {

Advanced

A wonderful way to get the bit-width of an integer type's value bits:

// https://stackoverflow.com/a/4589384/2410359
/* Number of value bits in inttype_MAX, or in any (1<<k)-1 where 0 <= k < 2040 */
#define IMAX_BITS(m) ((m)/((m)%255+1) / 255%255*8 + 7-86/((m)%255+12))

#define ULONG_WIDTH IMAX_BITS(ULONG_MAX)

Shifting more than "32"

To handle n outside the [1 ... ULONG_WIDTH) range, reduce the shift.

void rotateLeft(unsigned long num, int n) {
  // Bring n into (-ULONG_WIDTH ... ULONG_WIDTH) range,
  n %= ULONG_WIDTH;
  // Handle negative n.
  if (n < 0) n += ULONG_WIDTH;
  // Cope with 0 as a special case as `num >> (ULONG_WIDTH - 0)` is bad.
  unsigned long val = n == 0 ? n : (num << n) | (num >> (ULONG_WIDTH - n));
  bin_print(val);
  printf("%lu\n", val);
}

Not going to mention `sizeof`? I'm going to go out on a limb and suggest that _most_ modern architectures define a byte as 8 bits... `:-)` — Fe2O3, Jan 05 '23 at 13:03
@Fe2O3 A classic `#define ULONG_WIDTH (sizeof(unsigned long)*CHAR_BIT)` is reasonable, yet has 2 short-comings: that `ULONG_WIDTH` is not usable in prepossessing. `#define ULONG_WIDTH IMAX_BITS(ULONG_MAX)` is usable in PP. On rare machines with padding, `(sizeof(unsigned long)*CHAR_BIT)` is incorrect for OP's goal regardless of `CHAR_BIT`. — chux - Reinstate Monica, Jan 05 '23 at 13:08

Circular right and left shift in C

1 Answers1