following is a simple embedded c piece of code, can somebody tell me is there a problem in it ?

boolean new(int a)
{
      return !(a & 7);
}

i am new to embedded programming. It will be great help, any pointers to this ...

thanks
pdk

>can somebody tell me is there a problem in it ?
Assuming boolean is a typedef, there's nothing syntactically or semantically wrong with that function. Perhaps if you told us what it's supposed to do, we can tell you if it actually does it.

it was asked for me in the test. where they asked me
1.what the code does and
2. the potential problem with that.

i answered, If any of the last 3 lsb bits of the number is set to '1' return '0'. Else return 1. and i could not see any potential problem in that..

Your analysis is correct.

But whether there is anything 'wrong' depends on what it's supposed to do.
If for example is was a test to see if a number was >= 8, then it would fail miserably on negative numbers.

The name 'new' certainly doesn't help.

thankyou for the reply.
but i am not sure whether this logic will work ( i.e to check whether the number is >=8) .. as lets say for eg +9 , the program still returns '0'

True.
So what's the indented function then?
If we don't know what it's supposed to do, how can we say whether it's right or wrong?

even i was not sure. because in the test, i was supposed to tell what the function is intended to do and any potential problems !!!.
i could not exactly figure out this .. i.e why any pointers to this will be helpful.

as pointed out, i was also thinking whether it will cause any problems for -ve numbers .. but without any potential soln..

This function resave number, if the number is 0 its return true else its return false.
The Boolean " a & 7 ;" says if a != 0 and 7 != 0 then 1 else 0
Has you see the 7 is not needed

This function resave number, if the number is 0 its return true else its return false.
The Boolean " a & 7 ;" says if a != 0 and 7 != 0 then 1 else 0
Has you see the 7 is not needed

--------------------------------

when the number is lets say '8'. it still returns '1' .. i.e basically numbers for which lsb 3 bits are not set. it will return '1'

So knowing what it does, you can write a spec which means there isn't a problem, or you can write a different spec where there is a problem.

I suppose pedantically, the parameter should be unsigned, if you're messing around with bits.

Comments
Totally agreee with you!

Hmmmm that was good question, it need a 2 min think before we could answer that question. I never an interview question like that before. But that was good one.

I don't see any improvements when we could do to that code. Everything seem to be good. And I do think 7 needs to be there if you wanted check the last 3 bit of a to check if they are set.

And again it all depends upon the requirement!

ssharish

My guess:
1)The function checks whether the integer a is divisible by 8.
2)The use of new which is not an operator in c but in c++.

yes, this looks like the correct answer.

I was thinking that for -ve numbers this may fail. But this even checks for -ve numbers stored in 2's complenent form correctly i guess.

Then i donot see any potential problem in the function. except that function returns 1 for '0' also and all numbers (-ve and +ve ) divisible by 8.

now i got some part of this answer:
i have couple of other questions in the same form.

which i gave some answers, May be u can correct me , if i was wrong:

bool f( uint n )

{

  return (n & (n-1)) == 0;

}

My answer was : if n is powers of 2 then return 1 else '0'

uint f( uint n )

{

  return –n & 7;

}

My answer: last 3 (LSB) bits of the 2's complement of the input number .

int f( int n, int l, int r )

{

  return (n << l) >> r;

}

my Answer : n is bit shifted left by l bits and result is again right shifted by n bits.

void fn(long* p1, long* p2)

{

      register int x = *p1;

      register int y = *p2;

      x ^= y;

      y ^= x;

      x ^= y;

      *p1 = x;

      *p2 = y;

}

my answer : swaps the two inputs x and y.

void send(int count, short *to, short *from)

{

    /* count > 0 assumed */

    register n = (count + 7) / 8; 

    switch (count % 8)

    {

    case 0:        do {  *to = *from++;

    case 7:              *to = *from++;

    case 6:              *to = *from++;

    case 5:              *to = *from++;

    case 4:              *to = *from++;

    case 3:              *to = *from++;

    case 2:              *to = *from++;

    case 1:              *to = *from++;

                   } while (--n > 0);

    }

}

My answer : this expression copies the counth element of 'from' array to the first element of the 'to' array.

Edited 3 Years Ago by mike_2000_17: Fixed formatting

yes, i.e TRUE, for other representations of -ve numbers it will fail.

1

bool f( uint n )

{

return (n & (n-1)) == 0;

}

My answer was : if n is powers of 2 then return 1 else '0'

2.

uint f( uint n )

{

return –n & 7;

}

My answer: last 3 (LSB) bits of the 2's complement of the input number .

3.

int f( int n, int l, int r )

{

return (n << l) >> r;

}

my Answer : n is bit shifted left by l bits and result is again right shifted by n bits.
6.

void fn(long* p1, long* p2)

{

register int x = *p1;

register int y = *p2;

x ^= y;

y ^= x;

x ^= y;

*p1 = x;

*p2 = y;

}

my answer : swaps the two inputs x and y.

7.

void send(int count, short *to, short *from)

{

/* count > 0 assumed */

register n = (count + 7) / 8;

switch (count % 8)

{

case 0: do { *to = *from++;

case 7: *to = *from++;

case 6: *to = *from++;

case 5: *to = *from++;

case 4: *to = *from++;

case 3: *to = *from++;

case 2: *to = *from++;

case 1: *to = *from++;

} while (--n > 0);

}

}

My answer : this expression copies the counth element of 'from' array to the first element of the 'to' array.

6.

void fn(long* p1, long* p2)

{

register int x = *p1;

register int y = *p2;

x ^= y;

y ^= x;

x ^= y;

*p1 = x;

*p2 = y;

}

my answer : swaps the two inputs x and y.

XOR Swap - not recommended. More or less the same for register .

7.

void send(int count, short *to, short *from)

{

/* count > 0 assumed */

register n = (count + 7) / 8;

switch (count % 8)

{

case 0: do { *to = *from++;

case 7: *to = *from++;

case 6: *to = *from++;

case 5: *to = *from++;

case 4: *to = *from++;

case 3: *to = *from++;

case 2: *to = *from++;

case 1: *to = *from++;

} while (--n > 0);

}

}

My answer : this expression copies the counth element of 'from' array to the first element of the 'to' array.

Duff's Device

thanks Dave for the answers.

for 6) is it not a problem that register is 'int' type where as input is ptr to long int.

The input is not pointer to long int but long int.

When we declare int a;
the compiler assumes the following.
auto signed [long or short] int a ;

In that code you want it to be in registers,
but compiler can not assure you that as Dave said
As far as I know Only in Turbo c++ compiler int defaults to auto signed short int(May be because it is a 16-bit compiler). But in many other compilers int defaults to auto signed long int
That is why the code is correct

I may well be wrong though, You may wait for some experts to clarify.

The problem with the first function may be the problem pointed by Salem If 2's compliment is not used then the code is wrong.

This article has been dead for over six months. Start a new discussion instead.