Masks bits of array members

Question

Can you direct me where is my problem here? I am trying to test values of an array.

Every value presents 16 bits (flags) and I need to check if the flag is set or not.

My current output is shown below and also the expected output.

 #include <stdio.h>

    unsigned short value[1];
    unsigned int bitCheck(unsigned int mask, int pin);
    unsigned short mask;

int main(void){
    value[0]=0;
    value[1]=4095;


    int pin0 = 0;
    int pin1 = 1;

    unsigned int bit0= bitCheck( mask, pin0);
    unsigned int bit1= bitCheck( mask, pin1);


  for (int i =0;i<=1; i++)
{
    mask=value[i];
   printf("Mask = %d ==>>\n", mask);

   if ( bit0 == 1 ){
      printf("Pin %d is Set\n", pin0);
   }else{
      printf("Pin %d is not Set\n", pin0);
   }

    if ( bit1 == 1 ){
      printf("Pin %d is Set\n", pin1);
   }else{
      printf("Pin %d is not Set\n", pin1);
   }

   printf("\n"); 
}

}

unsigned int bitCheck(unsigned int mask, int bit){
   if ( (mask >> bit ) & 1){
      return 1;
   }else{
      return 0;
   }
}

My output is:

Mask = 0 ==>>
Pin 0 is not Set
Pin 1 is not Set

Mask = 4095 ==>>
Pin 0 is not Set
Pin 1 is not Set

and it must be:

Mask = 0 ==>>
Pin 0 is not Set
Pin 1 is not Set

Mask = 4095 ==>>
Pin 0 is Set
Pin 1 is Set

Umm, you didn't set `mask` originally, so it remains 0. Then later in the loop you're not setting `pin` variables. I am not sure *why* you think this should give any other results... — Antti Haapala -- Слава Україні, Aug 17 '17 at 06:13
Or, like my friend put it, perhaps there wasn't enough cosmic radiation to flip the bits in 0s to 1s. It might help if you produce an actual [mcve], with the emphasis on the word minimal. — Antti Haapala -- Слава Україні, Aug 17 '17 at 06:15
Please ident your code properly in the future. It is totally unreadable. — Nikolai Shalakin, Aug 17 '17 at 06:26
@MhdBanat yes, that's why it prints `4095` the second time... where are you re-setting the `pinN` then? — Antti Haapala -- Слава Україні, Aug 17 '17 at 06:28
The [mcve] means that you will have to remove the non-essential parts of the code ... in *your* program - then run it. The 12-time repetition is completely unnecessary to reproduce the problem. Had you done this, the answer would have been clear right away. — Antti Haapala -- Слава Україні, Aug 17 '17 at 06:29
Thank you all for correcting me, @AnttiHaapala, @ Nikolai Shalakin. It's now minimal, complete, verifiable example. What is my problem ? — MhdBanat, Aug 17 '17 at 06:58
You just need `printf` for one bit... and the `for` loop can go too... just set the value that you need... — Antti Haapala -- Слава Україні, Aug 17 '17 at 07:07

score 2 · Accepted Answer · 2017-08-17T07:21:16.323

This code is unnecessarily complex and through it's complexity, hides a blatant logical error. You're doing your calls to bitCheck(mask, ...) before you ever assign a value to mask.

Change this:

    unsigned int bit0= bitCheck( mask, pin0);
    unsigned int bit1= bitCheck( mask, pin1);


  for (int i =0;i<=1; i++)
{
    mask=value[i];
   printf("Mask = %d ==>>\n", mask);

to this:

  for (int i =0;i<=1; i++)
{
    mask=value[i];

    unsigned int bit0= bitCheck( mask, pin0);
    unsigned int bit1= bitCheck( mask, pin1);

   printf("Mask = %d ==>>\n", mask);

That said, here's a sane way to do all this:

#include <stdio.h>

#define BIT(n, v) (!!((v) & (1U << (n))))

int main(void)
{
    unsigned short values[] = {0, 4095};
    for (int i = 0; i < 2; ++i)
    {
        printf("value: %hu\n", values[i]);
        for (int bit = 0; bit < 16; ++bit)
        {
            printf( BIT(bit, values[i]) ?
                    "bit %d is set\n" :
                    "bit %d is not set\n", bit);
        }
    }
    return 0;
}

For one reason why your approach isn't considered sane, read about the DRY principle. Writing 16 times the same code is a simple example of violating this principle -> you need a loop instead.

On 16-bit arches, without `U` suffix even this would shift to the sign bit and therefore UB. — Antti Haapala -- Слава Україні, Aug 17 '17 at 07:11
@AnttiHaapala yes, for 16bit `int` you're correct of course. I changed the code. — , Aug 17 '17 at 07:11

Masks bits of array members

1 Answers1