xiaoxiang781216 commented on code in PR #15570:
URL: https://github.com/apache/nuttx/pull/15570#discussion_r1917600126


##########
include/nuttx/sensors/msxxxx_crc4.h:
##########
@@ -44,41 +44,40 @@ static uint8_t msxxxx_crc4(FAR uint16_t *src,
                            uint16_t crcmask)
 {
   uint16_t cnt;
-  uint16_t n_rem;
-  uint16_t crc_read;
+  uint16_t n_rem;     /* CRC remainder */
+  uint16_t crc_read;  /* Original value of the CRC */
   uint8_t n_bit;
 
   n_rem = 0x00;
-  crc_read = src[crcndx];
-  src[crcndx] &= ~crcmask;
+  crc_read = src[crcndx];   /* Save read CRC */
+  src[crcndx] &= ~crcmask;  /* CRC byte is replaced by 0 */
 
-  for (cnt = 0; cnt < 16; cnt++)
+  for (cnt = 0; cnt < 16; cnt++)  /* Operation is performed on bytes */
     {
       if (cnt % 2 == 1)
         {
-          n_rem ^= src[cnt >> 1] & 0x00ff;
+          n_rem ^= (uint16_t)((src[cnt >> 1]) & 0x00ff);
         }
       else
         {
-          n_rem ^= src[cnt >> 1] >> 8;
+          n_rem ^= (uint16_t)(src[cnt >> 1] >> 8);
         }
 
       for (n_bit = 8; n_bit > 0; n_bit--)
         {
-          if (n_rem & (0x8000) != 0)
+          if (n_rem & (0x8000))
             {
               n_rem = (n_rem << 1) ^ 0x3000;
             }
           else
             {
-              n_rem <<= 1;
+              n_rem = (n_rem << 1);

Review Comment:
   why change



##########
include/nuttx/sensors/msxxxx_crc4.h:
##########
@@ -44,41 +44,40 @@ static uint8_t msxxxx_crc4(FAR uint16_t *src,
                            uint16_t crcmask)
 {
   uint16_t cnt;
-  uint16_t n_rem;
-  uint16_t crc_read;
+  uint16_t n_rem;     /* CRC remainder */
+  uint16_t crc_read;  /* Original value of the CRC */
   uint8_t n_bit;
 
   n_rem = 0x00;
-  crc_read = src[crcndx];
-  src[crcndx] &= ~crcmask;
+  crc_read = src[crcndx];   /* Save read CRC */
+  src[crcndx] &= ~crcmask;  /* CRC byte is replaced by 0 */
 
-  for (cnt = 0; cnt < 16; cnt++)
+  for (cnt = 0; cnt < 16; cnt++)  /* Operation is performed on bytes */
     {
       if (cnt % 2 == 1)
         {
-          n_rem ^= src[cnt >> 1] & 0x00ff;
+          n_rem ^= (uint16_t)((src[cnt >> 1]) & 0x00ff);
         }
       else
         {
-          n_rem ^= src[cnt >> 1] >> 8;
+          n_rem ^= (uint16_t)(src[cnt >> 1] >> 8);

Review Comment:
   ditto



##########
include/nuttx/sensors/msxxxx_crc4.h:
##########
@@ -44,41 +44,40 @@ static uint8_t msxxxx_crc4(FAR uint16_t *src,
                            uint16_t crcmask)
 {
   uint16_t cnt;
-  uint16_t n_rem;
-  uint16_t crc_read;
+  uint16_t n_rem;     /* CRC remainder */
+  uint16_t crc_read;  /* Original value of the CRC */
   uint8_t n_bit;
 
   n_rem = 0x00;
-  crc_read = src[crcndx];
-  src[crcndx] &= ~crcmask;
+  crc_read = src[crcndx];   /* Save read CRC */
+  src[crcndx] &= ~crcmask;  /* CRC byte is replaced by 0 */
 
-  for (cnt = 0; cnt < 16; cnt++)
+  for (cnt = 0; cnt < 16; cnt++)  /* Operation is performed on bytes */
     {
       if (cnt % 2 == 1)
         {
-          n_rem ^= src[cnt >> 1] & 0x00ff;
+          n_rem ^= (uint16_t)((src[cnt >> 1]) & 0x00ff);

Review Comment:
   why need the cast



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to