mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "George Spelvin" <linux@horizon.com>
To: akpm@linux-foundation.org, arend@broadcom.com
Cc: error27@gmail.com, linux-kernel@vger.kernel.org,
	linux-wireless@vger.kernel.org, linux@horizon.com,
	linville@tuxdriver.com
Subject: Re: [RFC] lib: crc8: add new library module providing crc8 algorithm
Date: 24 May 2011 18:52:24 -0400	[thread overview]
Message-ID: <20110524225224.32163.qmail@science.horizon.com> (raw)
In-Reply-To: <1306268601-10729-1-git-send-email-arend@broadcom.com>

> V3:
> - added function crc8_create().
> - crc8_create() takes polynomial and bit direction as parameters.

May I suggest that crc8_create is A Stupid Idea.  Since the bit order
(and polynomial) will always be compile-time constants, just let the
call sites call crc8_create_[lm]sb_first() directly.

That way there's no need for the enum, and specifying something
other than lsb_first and msb_first is a compile-time error rather than
a run-time one.

It might be nice to add some const declarations, and use size_t
for the buffer length:
u8 crc8(u8 const *table, u8 const *pdata, size_t nbytes, u8 crc)
or
u8 crc8(u8 const table[256], u8 const *pdata, size_t nbytes, u8 crc)


Style points you might consider, but I do not consider essential:

Personally, when a function parameter is a pointer to
a fixed-size array, I prefer to declare it as
	void crc8_create_lsb_first(u8 table[256], u8 poly)
for better documentation, but that's your choice.

The CRC8_GOOD_VALUE could be explained if you like.  "If a CRC is inverted
before transmission, the CRC computed over the while (message+crc)
is _table[x], when x is the bit pattern of the modification (almost
always 0xff)."

Thanks!

  reply	other threads:[~2011-05-24 22:52 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-24 20:23 Arend van Spriel
2011-05-24 22:52 ` George Spelvin [this message]
2011-05-25  5:21   ` Arend van Spriel
2011-05-25  5:46     ` George Spelvin
2011-05-25  6:49       ` Arend van Spriel
  -- strict thread matches above, loose matches on Subject: below --
2011-05-22  8:08 Arend van Spriel
2011-05-22  8:59 ` Nicolas Kaiser
2011-05-22 12:49   ` Arend van Spriel
2011-05-21 20:49 Arend van Spriel
2011-05-21 22:37 ` Johannes Berg
2011-05-22  8:23   ` Arend van Spriel
2011-05-22 11:23 ` Alan Cox
2011-05-22 12:47   ` Arend van Spriel
2011-05-22 14:19     ` Alan Cox
2011-05-22 15:38       ` Arend van Spriel
2011-05-24  7:45       ` Arend van Spriel
2011-05-24  8:30         ` Arend van Spriel
2011-05-24  9:44         ` Alan Cox
2011-05-24 10:01           ` Arend van Spriel
2011-05-24 10:25             ` Rafał Miłecki
2011-05-24 10:40               ` Arend van Spriel
2011-05-24 10:27             ` Alan Cox
2011-05-24 10:38               ` Arend van Spriel
2011-05-24 12:57                 ` Alan Cox
2011-05-24 17:00                   ` Arend van Spriel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110524225224.32163.qmail@science.horizon.com \
    --to=linux@horizon.com \
    --cc=akpm@linux-foundation.org \
    --cc=arend@broadcom.com \
    --cc=error27@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®