0
\$\begingroup\$

Is this code correct? Is there any major improvement I can ad in terms of number of operations?

#include <stdio.h>

/* function that yields 1 when run on a machine that uses
arithmetic right shifts for int's and 0 otherwise */

/* use only << >> ! & ~ ^ and constants between 0x0 and 0xff*/

int int_what_shifts();

int main(){
    int_what_shifts();
}

int int_what_shifts(){

    int mask = (0x80 << 24);

    printf("%d\n",!!(mask>>31));

    return !!(mask >> 31);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Does the code what you expect to do? The question of you "Is this correct" should be answered by you. If the answer is yes => change question to can you review mine code, if answer is no => off topic. \$\endgroup\$ – chillworld Jun 4 '14 at 21:43
5
\$\begingroup\$

Replace 24 with (sizeof(int) - 1) * CHAR_BIT. Otherwise the code would fail on a non-32-bit integer machines. Also, using -1 as a probe makes you independent of the bitness of a system:

return ((-1) >> 1) == (-1);

PS: you don't want to print anything from such a specialized routine.

\$\endgroup\$
  • \$\begingroup\$ Strictly speaking, ((-1) >> 1) == (-1) won't work if the system uses sign-magnitude signed type, although we can hardly seen it in modern systems anymore \$\endgroup\$ – phuclv Jun 5 '14 at 11:02
  • \$\begingroup\$ @LưuVĩnhPhúc I know. It will also not work on 1-complement systems... \$\endgroup\$ – vnp Jun 5 '14 at 16:53
  • \$\begingroup\$ ah yes, don't know why I though that -1 in one's complement is also all 1 bits \$\endgroup\$ – phuclv Jun 6 '14 at 2:07
2
\$\begingroup\$

Your function always return 1 on machines with 32-bit int because mask >> 31 will return 1 or ~0 depending on that compiler uses arithmetic or logical shift right, and apply !! to mask >> 31 makes it return 1. Also, you should print the result of the function instead of printing in the function and discard the result.

Also note that people often use 1 << 31 rather than 0x80 << 24. And you need U suffix to make sure that it doesn't invoke undefined behavior due to overflow. (int)((signed char)0x80) << 24 will be OK in case CHAR_BIT == 8 but 0x80 << 24 won't, because 0x80 is a positive number in the range of int, and shifting it 24 bits cause the number to overflow

#define NUM_BITS (sizeof(int)*CHAR_BIT)

int mask = (int)(0x01U << (NUM_BITS - 1));    
return (mask >> (NUM_BITS - 1)) != 1;

But to be efficient, use

return ((~0) >> 1) == (~0);

instead. It'll work regardless of the signed type form because like I said in the other comment, vnp's solution won't work if the system uses sign-magnitude form.

Even better, define the function as macro or inline it.

\$\endgroup\$
  • \$\begingroup\$ ~0 may be a trap in one's complement (it is negative zero). Also, (int)(1u << (NUM_BITS-1)) may raise a signal due to out-of-range conversion: the unsigned expression is out of range for signed int. Same with (signed char)0x80. \$\endgroup\$ – M.M Jun 25 '14 at 21:25
1
\$\begingroup\$
  1. The premise is questionable. int mask = (0x80 << 24); Performing enough left shifts on an int on a positive number that exceeds INT_MAX is undefined behavior. C11dr §6.5.7 4

  2. In C, an int with a negative value right shifted is implementation-defined behavior. That is what this function appears to address with !!(mask >> 31);. Suggest not starting with the above UB.

    int int_what_shifts(void) {
      // Assume unsigned bit width completely covers int
      union {
        unsigned u;
        int i;
      } x;
      /* use only << >> ! & ~ ^ and constants between 0x0 and 0xff*/
      unsigned maxu = ~0u;  // all bits set
      x.u = maxu;
      x.i = x.i >> 1;
      // An arithmetic right shift on an all 1's pattern, results in all 1's
      return !(x.u ^ maxu);  // Test for equality
    }
    
  3. int_what_shifts() should be int_what_shifts(void).

  4. 24 and 31 are magic numbers that useless in C for this purpose. C does not define an int to be 32-bit 2's compliment. In C, ints are at least 16-bits and 64-bit `int are not unheard of.

\$\endgroup\$
  • \$\begingroup\$ Why don't you just do int i = ~0; ? If this is a trap then so is your access of x.i, and if it isn't then it's fine. \$\endgroup\$ – M.M Jun 25 '14 at 8:22
  • \$\begingroup\$ @Matt McNabb Agree that our 2 approaches would likely both set off a trap if the other did. The code above has the slight advantage if int i = ~0 was a trap and ` x.i = x.i >> 1;` did not cause a trap, things would work. Of course your approach has the advantage in a reverse case. \$\endgroup\$ – chux Jun 25 '14 at 17:48
  • \$\begingroup\$ I think reading x.i after doing x.u = ~0u; must get the same result as x.i = ~0;. All bits are set in both cases. So either approach traps if ~0 is a trap. However the standard is slightly vague on the behaviour of ~ , in other places it specifically says if a result may be undefined or implementation-defined ; but it doesn't say anything for ~. \$\endgroup\$ – M.M Jun 25 '14 at 21:18
  • \$\begingroup\$ @Matt McNabb Setting the trap issue aside, I heard tell of an unusual unsigned that had a range 0...INT_MAX. Thus your idea of int i = ~0 may be good in that case. \$\endgroup\$ – chux Jun 25 '14 at 22:15
  • \$\begingroup\$ I think that does not conform: unsigned int is specified as it must have a pure binary representation; and int must have a sign bit. \$\endgroup\$ – M.M Jun 25 '14 at 22:18
0
\$\begingroup\$

According to the C standard, doing this test invokes implementation-defined behaviour, and it need not be either of a logical shift or arithmetic shift!

Also, C allows padding bits. (For example, unsigned int could be 32-bit, but signed int could be 30 value bits, 1 sign bit, and 1 padding bit; and the padding bit could be a parity check bit positioned in the MSB.

Allowing for this is pretty annoying so let's assume we are on a system that's not 40 years old, and thus it has no padding bits in int; and also assume that the shift is either arithmetic or it isn't.

Then we can just go:

return (-1 >> 1) >= 0;

-1 has the sign bit set; on an arithmetic shift the sign bit will remain set afterwards; and on a logical shift the sign bit will become unset.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.