redrat3: remove memcpys and fix unaligned memory access

Hi Sean,

I had a question about 4c055a5ae94c: "[media] redrat3: remove memcpys and fix unaligned memory access" from Feb 16, 2013.

drivers/media/rc/redrat3.c

   619          /* grab the Length and type of transfer */
   620          pktlen = be16_to_cpu(header->length);
   621          pkttype = be16_to_cpu(header->transfer_type);
   622  
   623          if (pktlen > sizeof(rr3->irdata)) {
   624                  dev_warn(rr3->dev, "packet length %u too large\n", pktlen);
   625                  return;
   626          }
   627  
   628          switch (pkttype) {
   629          case RR3_ERROR:
   630                  if (len >= sizeof(struct redrat3_error)) {
   631                          struct redrat3_error *error = rr3->bulk_in_buf;
   632                          unsigned fw_error = be16_to_cpu(error->fw_error);
   633                          redrat3_dump_fw_error(rr3, fw_error);
   634                  }
   635                  break;
   636  
   637          case RR3_MOD_SIGNAL_IN:
   638                  memcpy(&rr3->irdata, rr3->bulk_in_buf, len);
                                                               ^^^
   639                  rr3->bytes_read = len;
                                          ^^^
   640                  rr3_dbg(rr3->dev, "bytes_read %d, pktlen %d\n",
   641                          rr3->bytes_read, pktlen);
                                                 ^^^^^^

Should we be copying "pktlen" bytes on the line before? It seems inconsistent that it doesn't match the debug code.

My main concern is that we limit the size of "pktlen" but then we only use it for debug output.

642 break;

regards,
dan carpenter

The pktlen is the length of the entire packet, and here we might have received only part of it. This debug line would have been better if it was:

	rr3_dbg(rr3->dev, "%s read of total %d\n", len, pktlen);

pktlen is a copy of the length field of the packet. That gets memcpy'd (line 638) into rr3->irdata where it is available as rr3->irdata.length. It is later used to check if we've received the full packet:

689 if (rr3->bytes_read < be16_to_cpu(rr3->irdata.header.length))
690 /* we're still accumulating data */
691 return 0;

HTH and thanks for checking my changes.

Sean