jinliangli commented on code in PR #16130:
URL: https://github.com/apache/nuttx/pull/16130#discussion_r2027213848
##########
libs/libc/misc/lib_crc32.c:
##########
@@ -18,110 +18,127 @@
* License for the specific language governing permissions and limitations
* under the License.
*
-
***********************************************************************************************/
+ ****************************************************************************/
/* The logic in this file was developed by Gary S. Brown:
*
- * COPYRIGHT (C) 1986 Gary S. Brown. You may use this program, or code or
tables
- * extracted from it, as desired without restriction.
+ * COPYRIGHT (C) 1986 Gary S. Brown. You may use this program, or code or
+ * tables extracted from it, as desired without restriction.
*
- * First, the polynomial itself and its table of feedback terms. The
polynomial is:
+ * First, the polynomial itself and its table of feedback terms. The
+ * polynomial is:
*
* X^32+X^26+X^23+X^22+X^16+X^12+X^11+X^10+X^8+X^7+X^5+X^4+X^2+X^1+X^0
*
- * Note that we take it "backwards" and put the highest-order term in the
lowest-order bit.
- * The X^32 term is "implied"; the LSB is the X^31 term, etc. The X^0 term
(usually shown
- * as "+1") results in the MSB being 1
- *
- * Note that the usual hardware shift register implementation, which is what
we're using
- * (we're merely optimizing it by doing eight-bit chunks at a time) shifts
bits into the
- * lowest-order term. In our implementation, that means shifting towards the
right. Why
- * do we do it this way? Because the calculated CRC must be transmitted in
order from
- * highest-order term to lowest-order term. UARTs transmit characters in
order from LSB
- * to MSB. By storing the CRC this way we hand it to the UART in the order
low-byte to
- * high-byte; the UART sends each low-bit to hight-bit; and the result is
transmission bit
- * by bit from highest- to lowest-order term without requiring any bit
shuffling on our
- * part. Reception works similarly
+ * Note that we take it "backwards" and put the highest-order term in the
+ * lowest-order bit. The X^32 term is "implied"; the LSB is the X^31 term,
+ * etc. The X^0 term (usually shown as "+1") results in the MSB being 1
+ *
+ * Note that the usual hardware shift register implementation, which is what
+ * we're using (we're merely optimizing it by doing eight-bit chunks at a
+ * time) shifts bits into the lowest-order term. In our implementation, that
+ * means shifting towards the right. Why do we do it this way? Because the
+ * calculated CRC must be transmitted in order from
+ * highest-order term to lowest-order term. UARTs transmit characters in
+ * order from LSB to MSB. By storing the CRC this way we hand it to the UART
+ * in the order low-byte to high-byte; the UART sends each low-bit to
+ * hight-bit; and the result is transmission bit by bit from highest- to
+ * lowest-order term without requiring any bit shuffling on our part.
+ * Reception works similarly
*
* The feedback terms table consists of 256, 32-bit entries. Notes
*
- * - The table can be generated at runtime if desired; code to do so is shown
later. It
- * might not be obvious, but the feedback terms simply represent the results
of eight
- * shift/xor operations for all combinations of data and CRC register values
+ * - The table can be generated at runtime if desired; code to do so is shown
+ * later. It might not be obvious, but the feedback terms simply represent
+ * the results of eight shift/xor operations for all combinations of data
+ * and CRC register values
*
- * - The values must be right-shifted by eight bits by the updcrc logic; the
shift must
- * be u_(bring in zeroes). On some hardware you could probably optimize the
shift in
- * assembler by using byte-swap instructions polynomial $edb88320
-
************************************************************************************************/
+ * - The values must be right-shifted by eight bits by the updcrc logic; the
+ * shift must be u_(bring in zeroes). On some hardware you could probably
+ * optimize the shift in assembler by using byte-swap instructions
+ * polynomial $edb88320
Review Comment:
NuttX orignal crc32part function use the Reversed Polynomial(0xedb88320)
and related lookup table, that can generate same results if we call it with
inital value(0xFFFFFFFF) and xor the final value with (0xFFFFFFFF). Just like
crypto dev crc32(ioctl) did. This is tested by testing/drivers/crypto/crc32.c.
However libc_crc32.c ::crc32 calls crc32part with inital value(0x0), and
doesn't xor the final value with 0xFFFFFFFF， that's not compatible with crc32
normal standard.
I can't decide if we should change libc_crc32.c :: crc32 api semantics to
follow standard. For example:
Change
`uint32_t crc32(FAR const uint8_t *src, size_t len)
{
return crc32part(src, len, 0);
}`
To
`uint32_t crc32(FAR const uint8_t *src, size_t len)
{
return crc32part(src, len, 0xFFFFFFFF) ^ 0xFFFFFFFF;
}`
Maybe that should be another PR if we decide to do that, and modify all
impacted apps.
--
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
