What's wrong with this code?

[xbyte]

It's a problem from a book, and I can't figure it out.

4 words (4x8 bits) are packed into a 32-bit unsigned integer, and
you've gotta extract an indicated (by bytenum) word from it, AND the
following code is said to be written by a failed programmer:

int xbyte(unsigned word, int bytenum)
{
return (word >(bytenum << 3)) & 0xFF;
}

IMHO, this is a perfect bulk of code, what's your opinion?

[Joachim Schmitz]

"xbyte" <ji********@gmail.comschrieb im Newsbeitrag
news:11**********************@50g2000hsm.googlegro ups.com...

It's a problem from a book, and I can't figure it out.

4 words (4x8 bits) are packed into a 32-bit unsigned integer, and
you've gotta extract an indicated (by bytenum) word from it, AND the
following code is said to be written by a failed programmer:

int xbyte(unsigned word, int bytenum)
{
return (word >(bytenum << 3)) & 0xFF;
}

IMHO, this is a perfect bulk of code, what's your opinion?

The function should return a byte (unsigned char), but does return an int.
You'd need to check whether bytenum is in the range of 0-3
There's no quarantee that unsigned (int) is 32 bit long

Bye, Jojo

[Malcolm McLean]

"xbyte" <ji********@gmail.comwrote in message
news:11**********************@50g2000hsm.googlegro ups.com...

It's a problem from a book, and I can't figure it out.

4 words (4x8 bits) are packed into a 32-bit unsigned integer, and
you've gotta extract an indicated (by bytenum) word from it, AND the
following code is said to be written by a failed programmer:

int xbyte(unsigned word, int bytenum)
{
return (word >(bytenum << 3)) & 0xFF;
}

IMHO, this is a perfect bulk of code, what's your opinion?

The programmer might have failed the course, but the code is good enough to
be used even in a production environment. Because it is extremely unlikely
that it can fail usefully.

However bytenum should be checked to be within the range 0-3, word should
arguably be an unsigned long to guarantee at least 32 bits. I say "arguably"
because in fact an extremely low level function like this needs to be
efficient, and a conversion from int to long might be exactly what you don't
need if it is not a nop.
The identifier "word" is a bit unfortunate, since it looks like it might be
a user-defined basic type.

int is the conventional return type for functions returning chars, by the
way. You could argue this is a bad convention, but it allows -1 on error.

--
Free games and programming goodies.
http://www.personal.leeds.ac.uk/~bgy1mm

[Richard]

Flash Gordon <sp**@flash-gordon.me.ukwrites:

Malcolm McLean wrote, On 29/09/07 21:03:

>>
"xbyte" <ji********@gmail.comwrote in message
news:11**********************@50g2000hsm.googlegr oups.com...

>>It's a problem from a book, and I can't figure it out.

4 words (4x8 bits) are packed into a 32-bit unsigned integer, and
you've gotta extract an indicated (by bytenum) word from it, AND the
following code is said to be written by a failed programmer:

int xbyte(unsigned word, int bytenum)
{
return (word >(bytenum << 3)) & 0xFF;
}

IMHO, this is a perfect bulk of code, what's your opinion?

The programmer might have failed the course, but the code is good
enough to be used even in a production environment.

Depends on the environment. I can certainly see it being rejected at
review for using a left shift where what is really meant is
multiplication.

Not if we are talking about shifting as opposed to multiplication. Using
the shift makes it obvious that we are shifting bits around. "*8" on the
other hand might be any arbitrary multiplication that just happens to be
a power of 2.

[Willem]

Richard wrote:
) Flash Gordon <sp**@flash-gordon.me.ukwrites:
)Malcolm McLean wrote, On 29/09/07 21:03:
)>>
)>"xbyte" <ji********@gmail.comwrote in message
)>news:11**********************@50g2000hsm.googleg roups.com...
)>>It's a problem from a book, and I can't figure it out.
)>>>
)>>4 words (4x8 bits) are packed into a 32-bit unsigned integer, and
)>>you've gotta extract an indicated (by bytenum) word from it, AND the
)>>following code is said to be written by a failed programmer:
)>>>
)>>int xbyte(unsigned word, int bytenum)
)>>{
)>> return (word >(bytenum << 3)) & 0xFF;
)>>}
)>>>
)>>IMHO, this is a perfect bulk of code, what's your opinion?
)>>>
)>The programmer might have failed the course, but the code is good
)>enough to be used even in a production environment.
)>
)Depends on the environment. I can certainly see it being rejected at
)review for using a left shift where what is really meant is
)multiplication.
)
) Not if we are talking about shifting as opposed to multiplication. Using
) the shift makes it obvious that we are shifting bits around. "*8" on the
) other hand might be any arbitrary multiplication that just happens to be
) a power of 2.

I direct your attention to what the code actually tries to do.
8 *is* an arbitrary multiplication that happens to be a power of two.
It could just as well have been 9.

To make the point more clear, a slightly more portable way to write
it would be:

return (word >(bytenum * CHAR_BIT)) & UCHAR_MAX;

Although I'm sure there are several issues with that as well.
SaSW, Willem
--
Disclaimer: I am in no way responsible for any of the statements
made in the above text. For all I know I might be
drugged or something..
No I'm not paranoid. You all think I'm paranoid, don't you !
#EOT

[=?iso-2022-kr?q?Harald_van_D=0E=29=26=0Fk?=]

On Sun, 30 Sep 2007 01:32:04 +0200, Richard wrote:

Flash Gordon <sp**@flash-gordon.me.ukwrites:

>Malcolm McLean wrote, On 29/09/07 21:03:

>>>
"xbyte" <ji********@gmail.comwrote in message
news:11**********************@50g2000hsm.googleg roups.com...
It's a problem from a book, and I can't figure it out.

4 words (4x8 bits) are packed into a 32-bit unsigned integer, and
you've gotta extract an indicated (by bytenum) word from it, AND the
following code is said to be written by a failed programmer:

int xbyte(unsigned word, int bytenum) {
return (word >(bytenum << 3)) & 0xFF;
}

IMHO, this is a perfect bulk of code, what's your opinion?

The programmer might have failed the course, but the code is good
enough to be used even in a production environment.

Depends on the environment. I can certainly see it being rejected at
review for using a left shift where what is really meant is
multiplication.

Not if we are talking about shifting as opposed to multiplication. Using
the shift makes it obvious that we are shifting bits around. "*8" on the
other hand might be any arbitrary multiplication that just happens to be
a power of 2.

If you were writing the code for a 9-bit char 36-bit int machine, you
might end up with something like

int xbyte(unsigned word, int bytenum) {
return (word >(bytenum * 9)) & 0x1FF;
}

"*8" is just any arbitrary multiplication that happens to be a power of
2, in this case.

[Flash Gordon]

Willem wrote, On 30/09/07 00:45:

Richard wrote:
) Flash Gordon <sp**@flash-gordon.me.ukwrites:
)Malcolm McLean wrote, On 29/09/07 21:03:
)>>
)>"xbyte" <ji********@gmail.comwrote in message
)>news:11**********************@50g2000hsm.googleg roups.com...

<snip>

)>> return (word >(bytenum << 3)) & 0xFF;

<snip>

)>The programmer might have failed the course, but the code is good
)>enough to be used even in a production environment.
)>
)Depends on the environment. I can certainly see it being rejected at
)review for using a left shift where what is really meant is
)multiplication.
)
) Not if we are talking about shifting as opposed to multiplication. Using
) the shift makes it obvious that we are shifting bits around. "*8" on the
) other hand might be any arbitrary multiplication that just happens to be
) a power of 2.

I direct your attention to what the code actually tries to do.
8 *is* an arbitrary multiplication that happens to be a power of two.
It could just as well have been 9.

Yes, that is indeed a correct expansion of my point, as was Harold's
response to Richard.

To make the point more clear, a slightly more portable way to write
it would be:

return (word >(bytenum * CHAR_BIT)) & UCHAR_MAX;

Although I'm sure there are several issues with that as well.

It depends on the real requirement. If you are dealing with some format
where you have four 8 bit numbers packed in to an integer type then
using 8 and 0xFF is correct, if the requirement is 4 C bytes then your
code is correct.
--
Flash Gordon

[Army1987]

On Sat, 29 Sep 2007 08:43:44 -0700, xbyte wrote:

It's a problem from a book, and I can't figure it out.

4 words (4x8 bits) are packed into a 32-bit unsigned integer, and
you've gotta extract an indicated (by bytenum) word from it, AND the
following code is said to be written by a failed programmer:

int xbyte(unsigned word, int bytenum)
{
return (word >(bytenum << 3)) & 0xFF;
}

IMHO, this is a perfect bulk of code, what's your opinion?

None. But if you replace `<< 3` with `* CHAR_BIT` and 0xFF with
UCHAR_MAX (these are in <limits.h>), you don't have to know how
big the word and the bytes are.
--
Army1987 (Replace "NOSPAM" with "email")
A hamburger is better than nothing.
Nothing is better than eternal happiness.
Therefore, a hamburger is better than eternal happiness.

[Peter J. Holzer]

On 2007-09-30 12:40, Army1987 <ar******@NOSPAM.itwrote:

On Sat, 29 Sep 2007 08:43:44 -0700, xbyte wrote:

>It's a problem from a book, and I can't figure it out.

4 words (4x8 bits) are packed into a 32-bit unsigned integer, and
you've gotta extract an indicated (by bytenum) word from it, AND the
following code is said to be written by a failed programmer:

int xbyte(unsigned word, int bytenum)
{
return (word >(bytenum << 3)) & 0xFF;
}

IMHO, this is a perfect bulk of code, what's your opinion?

None. But if you replace `<< 3` with `* CHAR_BIT` and 0xFF with
UCHAR_MAX (these are in <limits.h>), you don't have to know how
big the word and the bytes are.

Except that the requirement explicitely says "4x8 bits", not 4 x
CHAR_BIT bits. That may be an error in the requirement, but more likely
it is mandated by some external source, like a file format or a
communication protocol (in the real world - I have seen that this
problem is from "a book"). Then * 8 and & 0xFF will work correctly even
on machines with larger bytes, but * CHAR_BIT and UCHAR_MAX will not.

hp

--
_ | Peter J. Holzer | I know I'd be respectful of a pirate
|_|_) | Sysadmin WSR | with an emu on his shoulder.
| | | hj*@hjp.at |
__/ | http://www.hjp.at/ | -- Sam in "Freefall"