135u5 / tinyos-main

Automatically exported from code.google.com/p/tinyos-main
1 stars 0 forks source link

Mixed use of ntohs/htons in blip IPAddressP.nc #19

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Found by review.
Start with an example from todays SVN (there are several in this source)

    addr->s6_addr16[0] = htons(0xfe80);
    addr->s6_addr16[4] = htons(panid);
    addr->s6_addr16[5] = ntohs(0x00FF);
    addr->s6_addr16[6] = ntohs(0xFE00);
    addr->s6_addr16[7] = htons(saddr);

As you can see in this code both htons(0xfe80) and ntohs(0x00FF) are used on 
what would be native constants.

htons and ntohs will always have the same implementation (either both will swap 
or not). So there will be no runtime error only confusion when reading...

Original issue reported on code.google.com by rogjo...@gmail.com on 18 Feb 2011 at 8:49

GoogleCodeExporter commented 8 years ago

Original comment by philip.l...@gmail.com on 21 Feb 2011 at 8:18

GoogleCodeExporter commented 8 years ago

Original comment by philip.l...@gmail.com on 21 Feb 2011 at 8:18

GoogleCodeExporter commented 8 years ago
I remember this from a while back -- I think what you're quoting is on lines 
42-48, which looks different correct to me in the repo (both the branch and 
tinyos trunk).  The use at 91 and 92 however looks like they should be htons 
instead.  

It seems right in both trunk/ and the blip-rpl-devel branch -- maybe I'm 
missing it?  I did a bunch of work on the addressing code that appears in the 
branch to properly form link-local addresses with the U/L bit.

Original comment by sdh...@gmail.com on 21 Feb 2011 at 8:31

GoogleCodeExporter commented 8 years ago
How very strange... I am sure that I used web access and was looking at trunk...
Anyway - here is another example furter down in the code
with ntohs on a constant

 if (addr->s6_addr16[0] == htons(0xfe80)) {
      // link-local
      if (m_short_addr && 
          addr->s6_addr16[5] == ntohs(0x00FF) &&
          addr->s6_addr16[6] == ntohs(0xFE00)) {
        if (ntohs(addr->s6_addr16[4]) == panid && 
            ntohs(addr->s6_addr16[7]) == saddr) {

Original comment by rogjo...@gmail.com on 21 Feb 2011 at 11:51

GoogleCodeExporter commented 8 years ago
Agreed there.  just committed to my branch; will be merging with core 
relatively soon (next week-ish) so it will get merged in then and avoid any 
conflicts.  Changed to fixed; you can move it back if you find other instances.

Thanks.

Original comment by sdh...@gmail.com on 22 Feb 2011 at 12:28