Path: csiph.com!weretis.net!feeder8.news.weretis.net!eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail
From: Ben Bacarisse <ben.usenet@bsb.me.uk>
Newsgroups: comp.lang.c
Subject: Re: Explain function
Date: Sun, 20 Aug 2023 14:39:17 +0100
Organization: A noiseless patient Spider
Lines: 55
Message-ID: <877cppvnne.fsf@bsb.me.uk>
References: <00808c30-b47e-4ee2-b1da-5811e242e6cbn@googlegroups.com> <ubsvi8$1bhkn$1@dont-email.me>
MIME-Version: 1.0
Content-Type: text/plain
Injection-Info: dont-email.me; posting-host="e5d8f68d80d0d28048106bb566360eff"; logging-data="1454848"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX1+gR8Io2H+Je82npX0IUCpUtVy5AOPd0Qw="
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
Cancel-Lock: sha1:CDImr56/byqYvhbUytgMvRjnXTM= sha1:8jrvO4/a/RYdgIAWpag5MH2qzT0=
X-BSB-Auth: 1.e4e03f094104f418cdc9.20230820143917BST.877cppvnne.fsf@bsb.me.uk
Xref: csiph.com comp.lang.c:172577

David Brown <david.brown@hesbynett.no> writes:

> On 20/08/2023 11:46, Denis U wrote:
>> Hi
>> there is a function generating checksum:
>> unsigned short foo (unsigned short *addr, int len) {
>>      unsigned short result;
>>      unsigned int sum = 0;
>>     while (len > 1) {
>>          sum += *addr++;
>>          len -= 2;
>>      }
>>     if (len == 1) {
>>          sum += *(unsigned char *) addr;
>>      }
>>      // some other stuff
>> }
>> which I suppose to be called like
>>   unsigned short addr = 0xabcd;
>>   foo(&a, 5);
>> having unsigned short size equals 2 bytes
>> I don't understand why the function is going to iterate over other bytes more than 2. Where could those extra bytes come from?
>> 
>
> The checksum is not for a 2-byte short - it is for checksuming over an
> array of 2-byte short values.

Given the check for an odd length, it's clearly intended to produce a
16-bit checksum for an arbitrary object of any length -- not just for
some array of short values.

But if that is the case (and what's the extra test for otherwise) then
it's a badly written.  short is not a predictable length (but the
function may date from before the days of uint16_t) and there are
alignment issues.

To the OP: where is this from?  Are you supposed to be correcting it, or
is this supposed to be an example of good C from which you should learn?

Consider using something like this:

  uint_least16_t checksum(const char *addr, size_t len)
  {
       uint_fast16_t sum = 0;
       while (len > 1) {
            uint16_t tmp;
            memcpy(&tmp, addr, sizeof tmp);
            sum += tmp;
            len -= 2;
       }
       return sum + (len ? *(unsigned char *)addr : 0);
  }

-- 
Ben.