Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #172569 > unrolled thread

Explain function

Back to article view | Back to comp.lang.c

Contents

  Explain function Denis U <ua.kiev.energetix@gmail.com> - 2023-08-20 02:46 -0700
    Re: Explain function David Brown <david.brown@hesbynett.no> - 2023-08-20 14:05 +0200
      Re: Explain function Ben Bacarisse <ben.usenet@bsb.me.uk> - 2023-08-20 14:39 +0100
        Re: Explain function David Brown <david.brown@hesbynett.no> - 2023-08-20 16:10 +0200
          Re: Explain function Ben Bacarisse <ben.usenet@bsb.me.uk> - 2023-08-20 16:07 +0100
            Re: Explain function David Brown <david.brown@hesbynett.no> - 2023-08-20 18:04 +0200
              Re: Explain function Ben Bacarisse <ben.usenet@bsb.me.uk> - 2023-08-20 20:52 +0100
        Re: Explain function Malcolm McLean <malcolm.arthur.mclean@gmail.com> - 2023-08-20 07:20 -0700
          Re: Explain function Spiros Bousbouras <spibou@gmail.com> - 2023-08-20 14:38 +0000
        Re: Explain function Tim Rentsch <tr.17687@z991.linuxsc.com> - 2023-08-20 07:38 -0700

#172569 — Explain function

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?

[toc] | [next] | [standalone]

#172572

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.  Checksums are typically used for 
integrity checks for data that is stored or transmitted on networks, 
serial lines, etc.  The idea is that you transmit the data and the 
checksum, and on reception you can re-calculate the checksum from the 
received data and compare to the received checksum.  If there is a 
mismatch, something has been corrupted in the transfer - if there is a 
match, the transfer was probably fine.

So you would call "foo" as :

	unsigned short data[] = { 0xabcd, 0x1234, 0x9421 };
	unsigned short cs = foo(data, 6);

[toc] | [prev] | [next] | [standalone]

#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.

[toc] | [prev] | [next] | [standalone]

#172578

On 20/08/2023 15:39, Ben Bacarisse wrote:
> 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.

I agree entirely - I just wanted to keep things simple at the start, and 
then see where the OP wanted to go from there.

> 
> 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);
>    }
> 

If someone is learning C, I'd assume they are interested in more 
practical C.  Types like "uint_least16_t" have theoretical advantages 
over the fixed size types, but are almost never seen in real-world C 
code.  If the OP is interested in C as a language and knowing more about 
it, then more detail will be appropriate.  Even then, I'd question the 
appropriateness of returning uint_least16_t here.  (And did you skip 
incrementing addr as a debugging exercise for the OP? :-) )

uint16_t checksum_16bit_le(const uint8_t *addr, size_t len)
{
     uint16_t sum = 0;
     while (len > 1) {
          uint16_t tmp = *addr++;
          tmp += (*addr++) << 8;
          sum += tmp;
          len -= 2;
     }
     if (len) {
         sum += *addr;
     }
     return sum;
}


This version gives the same checksum on little-endian and big-endian 
processors, unlike the original one - but that is almost certainly what 
you want for a checksum, as the checksum algorithm is usually defined 
independently of the system it is run on.

And if the OP wants to know about good checksum algorithms, this is not 
a good one.

[toc] | [prev] | [next] | [standalone]

#172585

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

> On 20/08/2023 15:39, Ben Bacarisse wrote:
>> 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.
>
> I agree entirely - I just wanted to keep things simple at the start, and
> then see where the OP wanted to go from there.

I suspect it's a drive-by posting.

>> 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);
>>    }
>> 
>
> If someone is learning C, I'd assume they are interested in more practical
> C.  Types like "uint_least16_t" have theoretical advantages over the fixed
> size types, but are almost never seen in real-world C code.  If the OP is
> interested in C as a language and knowing more about it, then more detail
> will be appropriate.  Even then, I'd question the appropriateness of
> returning uint_least16_t here.  (And did you skip incrementing addr as a
> debugging exercise for the OP? :-) )

No, just tying off the top of my head.

> uint16_t checksum_16bit_le(const uint8_t *addr, size_t len)
> {
>     uint16_t sum = 0;
>     while (len > 1) {
>          uint16_t tmp = *addr++;
>          tmp += (*addr++) << 8;
>          sum += tmp;
>          len -= 2;
>     }
>     if (len) {
>         sum += *addr;
>     }
>     return sum;
> }
>
>
> This version gives the same checksum on little-endian and big-endian
> processors, unlike the original one - but that is almost certainly what you
> want for a checksum, as the checksum algorithm is usually defined
> independently of the system it is run on.

There's no specification.  It's all guesswork about what sort of
checksum this is.  I agree they are often externally defined, but they
are never externally definedto be what the OP's code implements, so who
knows what's wanted?

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#172588

On 20/08/2023 17:07, Ben Bacarisse wrote:

> There's no specification.  It's all guesswork about what sort of
> checksum this is.  I agree they are often externally defined, but they
> are never externally definedto be what the OP's code implements, so who
> knows what's wanted?
> 

I guess we will just have to wait and see if he comes back - then we'll 
hopefully know much more.

[toc] | [prev] | [next] | [standalone]

#172597

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

> On 20/08/2023 17:07, Ben Bacarisse wrote:
>
>> There's no specification.  It's all guesswork about what sort of
>> checksum this is.  I agree they are often externally defined, but they
>> are never externally definedto be what the OP's code implements, so who
>> knows what's wanted?
>
> I guess we will just have to wait and see if he comes back - then we'll
> hopefully know much more.

I'd like to know the context.  Is it "the teacher gave up this function
to use" or is it "I work for the MOD on missile control systems and I
have to write a checksum function"?

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#172580

On Sunday, 20 August 2023 at 14:39:34 UTC+1, Ben Bacarisse wrote:
> David Brown <david...@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); 
> } 
> 
unsigned short checksum(const unsigned char *addr, int len)
{
    unsigned short sum  = 0;
    int i;
    for ( i = 0; i <len-1; i+ 2)
   {
       sum += addr[i+1] * 256 + addr[i];
   }
   if (i < len)
      sum += addr[i];

   return sum & 0xFFFF;
}

The question is what to do for endiannness.
Likely you really want the same result, regardless of the endianness of the platform.
And adding the last byte as a low value implies little-endian.

[toc] | [prev] | [next] | [standalone]

#172583

On Sun, 20 Aug 2023 07:20:07 -0700 (PDT)
Malcolm McLean <malcolm.arthur.mclean@gmail.com> wrote:
> unsigned short checksum(const unsigned char *addr, int len)
> {
>     unsigned short sum  = 0;
>     int i;
>     for ( i = 0; i <len-1; i+ 2)

i += 2

>    {
>        sum += addr[i+1] * 256 + addr[i];
>    }
>    if (i < len)
>       sum += addr[i];
> 
>    return sum & 0xFFFF;
> }

Regarding  unsigned short sum  = 0;

You want  unsigned int  not  unsigned short ; the latter may be promoted
to  int  for the arithmetic in which case your code will likely cause
undefined behaviour. For the same reason you want

    sum += (unsigned int) addr[i+1] * 256 + addr[i];

> The question is what to do for endiannness. Likely you really want the same
> result, regardless of the endianness of the platform. And adding the last
> byte as a low value implies little-endian.

[toc] | [prev] | [next] | [standalone]

#172584

Ben Bacarisse <ben.usenet@bsb.me.uk> writes:

> 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);
>   }

Bleah.

unsigned short
checksum( const char *bytes, size_t n ){
    unsigned short t;
    size_t k = sizeof t;
    unsigned r;

    for(  r = 0;  n >= k;  bytes += k,  n -= k  ){
	memcpy( &t, bytes, k );
	r += t;
    }

    if(  n > 0  ){
	t = 0;
	memcpy( &t, bytes, n  );
	r += t;
    }

    return  r;   
}

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web