tags:

views:

82

answers:

4

Please help me understand 2 things I found in this C code:

First, there is the whole code:

usbMsgLen_t usbFunctionSetup(uchar data[8])  
{  
usbRequest_t    *rq = (void *)data;  
static uchar    dataBuffer[4];  /* buffer must stay valid when usbFunctionSetup returns */  
if(rq->bRequest == CUSTOM_RQ_ECHO){ /* echo -- used for reliability tests */  
        dataBuffer[0] = rq->wValue.bytes[0];  
        dataBuffer[1] = rq->wValue.bytes[1];  
        dataBuffer[2] = rq->wIndex.bytes[0];  
        dataBuffer[3] = rq->wIndex.bytes[1];  
        usbMsgPtr = dataBuffer;         /* tell the driver which data to return */  
        return 4;  
    }else if(rq->bRequest == CUSTOM_RQ_SET_STATUS){  
        if(rq->wValue.bytes[0] & 1){    /* set LED */  
            LED_PORT_OUTPUT |= _BV(LED_BIT);  
        }else{                          /* clear LED */  
            LED_PORT_OUTPUT &= ~_BV(LED_BIT);  
        }  
    }else if(rq->bRequest == CUSTOM_RQ_GET_STATUS)  {  
        dataBuffer[0] = ((LED_PORT_OUTPUT & _BV(LED_BIT)) != 0);  
        usbMsgPtr = dataBuffer;         /* tell the driver which data to return */  
        return 1;                       /* tell the driver to send 1 byte */  
    }  
    return 0;   /* default for not implemented requests: return no data back to host */  
}  

Now, usbFunctionSetup gets array of 8 unsigned chars. Now there comes the line:

usbRequest_t    *rq = (void *)data;

So, I get the left side of the statement, but what is on the right? I know that (void *) is cast to this type, but why?

And second question is, isnt this code inefficient? Because first function receives 8 bytes of data, and than it creates additional pointer to them. And that additional pointer is created, at least if I am right, just to be able to access individual data by its name defined in usbRequest_t struct. Wouldn't be simpler and more efficient to just use in code instead of rq->bRequest == something just for example data[2]==something or if bRequest is bigger than one byte, for example data[1] == low_byte_of_something && data[2]== high_byte_of_something?

Thanks.

A: 

The function is getting a 'raw' buffer of data, and the line:

usbRequest_t    *rq = (void *)data;

is just creating a pointer to that buffer to access it using the data layout specified by the usbRequest_t struct.

There's absolutely nothing expensive about this operation - the compiler will likely not even actually create a new pointer variable in an optimized build.

On there other hand, there may well be portability concerns, but that might not be important for your particular application.

Michael Burr
A: 

Question 1: Since data is uchar *, you need to cast it to another type. You could cast it directly to usbRequest_t * if you wanted to.

Question 2: The address of rq->bRequest takes as much time to calculate as the address of data[2]. In both cases, you take a pointer off of the stack and add a fixed offset to it. Using the struct pointer results in clearer code.

tomlogic
A: 

your first line (rq = (void) ..) is telling the compiler to treat the 8 bytes passed as a usbRequest_t

presumably this is a packet from a device or network and the code wants to look at it as though it matched the structure

yes it creates a pointer - but what will almost certainly happen in the generated code is that the address of the first byte is simply loaded into a register. So there is no overhead

conversely your suggestion of rq->bRequest == something will actually copy the data - which is a (v small for 8 bytes) overhead

This is a very common technique for mapping structs onto serialized data streams

pm100
A: 

Regarding your claim that the code is inefficient, I see little or no difference between accessing data[x] and rq->x. In both cases, the code starts with a base-address, and offsets a few bytes (apparently to a maximum of 8 bytes).

[Base Address] + [Offset] is going to be the same, regardless of if you do it by array or by structure. And if simple additions are the source of inefficiency in your program, you have much bigger problems.

abelenky