Unix Programming - Whats going on here?

Author

Whats going on here?

jimi_xyz@hotmail.com

2006-12-18, 7:22 pm

Hi,
I have this peice of code, and I was wondering if someone could explain
to me what is going on here. Here is the section of the code, that is
confusing me..

//----------------------------------------------------

char lowrequest[] = LOWREQUEST;
unsigned char msg[MSGSIZE];

/*THESE NEXT LINE IS VERY CONFUSING*/
comm_t* cmd = (comm_t*) msg;
bzero(msg, MSGSIZE);
memcpy(msg, lowrequest, sizeof(lowrequest));

cmd->cmd = RESET

//--------------------------------------------------

If you need anymore information, just send me a email or post. Also any
help will be appreciated.

Thanks,
Jimmie

toby

2006-12-18, 7:22 pm

jimi_xyz@hotmail.com wrote:
> Hi,
> I have this peice of code, and I was wondering if someone could explain
> to me what is going on here. Here is the section of the code, that is
> confusing me..
>
>
> //----------------------------------------------------
>
> char lowrequest[] = LOWREQUEST;
> unsigned char msg[MSGSIZE];
>
> /*THESE NEXT LINE IS VERY CONFUSING*/
> comm_t* cmd = (comm_t*) msg;
> bzero(msg, MSGSIZE);
> memcpy(msg, lowrequest, sizeof(lowrequest));

This merely zeroes out a buffer msg[] of MSGSIZE bytes and copies the
initialised character string lowrequest[] to the start of it (an
automatic initialiser here is a bit silly). It's not particularly safe
though, will likely blow up if sizeof(lowrequest) > MSGSIZE.

>
> cmd->cmd = RESET
>
> //--------------------------------------------------
>
> If you need anymore information, just send me a email or post. Also any
> help will be appreciated.
>
> Thanks,
> Jimmie

Eric Sosman

2006-12-18, 7:22 pm

jimi_xyz@hotmail.com wrote On 12/18/06 16:50,:
> Hi,
> I have this peice of code, and I was wondering if someone could explain
> to me what is going on here. Here is the section of the code, that is
> confusing me..
>
>
> //----------------------------------------------------
>
> char lowrequest[] = LOWREQUEST;
> unsigned char msg[MSGSIZE];
>
> /*THESE NEXT LINE IS VERY CONFUSING*/
> comm_t* cmd = (comm_t*) msg;
> bzero(msg, MSGSIZE);
> memcpy(msg, lowrequest, sizeof(lowrequest));
>
> cmd->cmd = RESET
>
> //--------------------------------------------------
>
> If you need anymore information, just send me a email or post. Also any
> help will be appreciated.

Either there is a good deal more to the code, or it
was written by a sloppy programmer -- probably both.

The first "confusing" line takes a pointer to the
first character of msg, converts it to a pointer to a
comm_t object (probably a typedef of some kind), and
stores the converted pointer in cmd. (See below.)

The second confusing line fills the msg array with
bytes whose value is zero. This may or may not be useful;
at best, it is partly useful.

The third confusing line copies the entire contents
of the lowrequest array to the beginning of the msg array.
If lowrequest is shorter than MSGSIZE this overwrites the
recently-zeroed beginning of msg, so the effort spent
zeroing that part of msg was wasted (but the effort spent
clearing the tail end may have been useful; can't tell
from the fragment at hand). If the size of lowrequest is
exactly equal to MSGSIZE, then all of the cleared bytes are
overwritten and the bzero was a complete waste of time. And
if lowrequest is longer than MSGSIZE the program is trying
to put ten pounds of organic fertilizer in a one-pound sack,
with unpredictable but probably unpleasant consequences.

The final line shows by its syntax that cmd must be a
pointer to a struct or to a union, hence comm_t must be
a struct or a union type. But although cmd is supposed to
be a pointer to a comm_t, in fact it points to msg; the
programmer is trying to "reinterpret" the contents of msg
as a comm_t. This may or may not work, since C does not
guarantee that a miscellaneous char array like msg is
located at an address where a comm_t could begin; many
machines require that some of their data types be "aligned"
to particular addresses, and the code at hand risks
violating such a requirement.

As I say, there's probably a good deal more to the code
than you have shown, and it's possible that the programmer
simply "knows" that lowrequest will fit in msg, for example.
But it's highly likely that a cleaner and more robust piece
of code would work at least as well, if not better. That's
why I suspect a sloppy programmer somewhere in the background.

--
Eric.Sosman@sun.com

Ulrich Eckhardt

2006-12-18, 7:22 pm

jimi_xyz@hotmail.com wrote:
> char lowrequest[] = LOWREQUEST;

Create a char array, size and content determined by the macro LOWREQUEST.

> unsigned char msg[MSGSIZE];

Create an unsigned char array.

> comm_t* cmd = (comm_t*) msg;

Create a pointer, initialise it with the address of msg. There is no
address-of operator (&) here, but since msg is an array it decays into a
pointer here.

> bzero(msg, MSGSIZE);

Zero-fill msg.

> memcpy(msg, lowrequest, sizeof(lowrequest));

Copy the array lowrequest into the array msg.

> cmd->cmd = RESET

Missing semicolon. Anyhow, write to the array msg via the bastard pointer
cmd.

This code severely stinks for several reasons:
- lowrequest is almost unused. The author could as well have copied the
content of LOWREQUEST directly into msg.
- Using an array to create a local temporary object of a different type is
at least twisted. The only good explanation I could imagine is that it is
a structure containing an unbounded array as last element, i.e. an array
of size zero or one plus allocated memory for further storage. Hacks like
that need to be properly documented.
- Using casts usually only hides errors.
- bzero is deprecated, use memset instead.
- If you want to zero a local array, use '={0}' as initialiser.
- Zero-filling something and then copying something over it is pretty
redundant.
- sizeof is an operator, not a function. The invocation above has useless
brackets.

One note still: all the above has nothing to do with Unix as per the
group's topic. I'd take this to e.g. alt.comp.lang.learn.c-c++.

Uli

--
http://www.erlenstar.demon.co.uk/unix/

jimi_xyz@hotmail.com

2006-12-18, 7:22 pm

Ulrich Eckhardt wrote:
> jimi_xyz@hotmail.com wrote:
>
> Create a char array, size and content determined by the macro LOWREQUEST.
>
>
> Create an unsigned char array.
>
>
> Create a pointer, initialise it with the address of msg. There is no
> address-of operator (&) here, but since msg is an array it decays into a
> pointer here.
>
>
> Zero-fill msg.
>
>
> Copy the array lowrequest into the array msg.
>
>
> Missing semicolon. Anyhow, write to the array msg via the bastard pointer
> cmd.
>
> This code severely stinks for several reasons:
> - lowrequest is almost unused. The author could as well have copied the
> content of LOWREQUEST directly into msg.
> - Using an array to create a local temporary object of a different type is
> at least twisted. The only good explanation I could imagine is that it is
> a structure containing an unbounded array as last element, i.e. an array
> of size zero or one plus allocated memory for further storage. Hacks like
> that need to be properly documented.
> - Using casts usually only hides errors.
> - bzero is deprecated, use memset instead.
> - If you want to zero a local array, use '={0}' as initialiser.
> - Zero-filling something and then copying something over it is pretty
> redundant.
> - sizeof is an operator, not a function. The invocation above has useless
> brackets.
>
> One note still: all the above has nothing to do with Unix as per the
> group's topic. I'd take this to e.g. alt.comp.lang.learn.c-c++.
>
> Uli
>
> --
> http://www.erlenstar.demon.co.uk/unix/

Eh everyone thank you for your answers, helps out allot..
~Jimmie

jimi_xyz@hotmail.com

2006-12-18, 7:22 pm