You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/03/02 11:12:19 UTC

[GitHub] [mynewt-nimble] prasad-alatkar opened a new pull request #765: nimble/host: Fix check for valid static random address

prasad-alatkar opened a new pull request #765: nimble/host: Fix check for valid static random address
URL: https://github.com/apache/mynewt-nimble/pull/765
 
 
   As per spec v5.1, Vol 6, part B, section 1.3.2.1, random part of of static address should follow:
   • At least one bit of the random part of the address shall be 0
   • At least one bit of the random part of the address shall be 1  
   
   In my previous PR: [PR!435](https://github.com/apache/mynewt-nimble/pull/435), I had checked all bits of random address but only random part should have been addressed.  

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #765: nimble/host: Fix check for valid static random address

Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #765: nimble/host: Fix check for valid static random address
URL: https://github.com/apache/mynewt-nimble/pull/765#discussion_r388152582
 
 

 ##########
 File path: nimble/host/src/ble_hs_id.c
 ##########
 @@ -58,14 +58,20 @@ ble_hs_id_set_rnd(const uint8_t *rnd_addr)
 {
     uint8_t addr_type_byte;
     int rc;
-    uint8_t all_zeros[BLE_DEV_ADDR_LEN] = {0}, all_ones[BLE_DEV_ADDR_LEN] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
+    int i, rnd_part_sum = 0;
 
 
 Review comment:
   I'd make those checks explicit so that it obvious what is being check.
   Something like this maybe?
   
   ```
   int
   ble_hs_id_set_rnd(const uint8_t *rnd_addr)
   {
       uint8_t addr_type_byte;
       int ones;
       int rc;
   
       /* Make sure random part of rnd_addr is not all ones or zeros */
       addr_type_byte = rnd_addr[5] & 0xc0;
   
       /* count bits set to 1 in address */
       ones = __builtin_popcount(rnd_addr[0]);
       ones += __builtin_popcount(rnd_addr[1]);
       ones += __builtin_popcount(rnd_addr[2]);
       ones += __builtin_popcount(rnd_addr[3]);
       ones += __builtin_popcount(rnd_addr[4]);
       ones += __builtin_popcount(rnd_addr[5] & 0x3f);
   
       if ((addr_type_byte != 0x00 && addr_type_byte != 0xc0) ||
               (ones == 0 || ones == 46)) {
           return BLE_HS_EINVAL;
       }
   
       ble_hs_lock();
       rc = ble_hs_hci_util_set_random_addr(rnd_addr);
       if (rc == 0) {
           memcpy(ble_hs_id_rnd, rnd_addr, 6);
       }
       ble_hs_unlock();
   
       return rc;
   }
   ```
   
   
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #765: nimble/host: Fix check for valid static random address

Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #765: nimble/host: Fix check for valid static random address
URL: https://github.com/apache/mynewt-nimble/pull/765#discussion_r388152582
 
 

 ##########
 File path: nimble/host/src/ble_hs_id.c
 ##########
 @@ -58,14 +58,20 @@ ble_hs_id_set_rnd(const uint8_t *rnd_addr)
 {
     uint8_t addr_type_byte;
     int rc;
-    uint8_t all_zeros[BLE_DEV_ADDR_LEN] = {0}, all_ones[BLE_DEV_ADDR_LEN] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
+    int i, rnd_part_sum = 0;
 
 
 Review comment:
   I'd make those checks explicit so that it obvious what is being check.
   Something like this maybe?
   
   int
   ble_hs_id_set_rnd(const uint8_t *rnd_addr)
   {
       uint8_t addr_type_byte;
       int ones;
       int rc;
   
       /* Make sure random part of rnd_addr is not all ones or zeros */
       addr_type_byte = rnd_addr[5] & 0xc0;
   
       /* count bits set to 1 in address */
       ones = __builtin_popcount(rnd_addr[0]);
       ones += __builtin_popcount(rnd_addr[1]);
       ones += __builtin_popcount(rnd_addr[2]);
       ones += __builtin_popcount(rnd_addr[3]);
       ones += __builtin_popcount(rnd_addr[4]);
       ones += __builtin_popcount(rnd_addr[5] & 0x3f);
   
       if ((addr_type_byte != 0x00 && addr_type_byte != 0xc0) ||
               (ones == 0 || ones == 46)) {
           return BLE_HS_EINVAL;
       }
   
       ble_hs_lock();
       rc = ble_hs_hci_util_set_random_addr(rnd_addr);
       if (rc == 0) {
           memcpy(ble_hs_id_rnd, rnd_addr, 6);
       }
       ble_hs_unlock();
   
       return rc;
   }
   
   
   
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] sjanc commented on a change in pull request #765: nimble/host: Fix check for valid static random address

Posted by GitBox <gi...@apache.org>.
sjanc commented on a change in pull request #765: nimble/host: Fix check for valid static random address
URL: https://github.com/apache/mynewt-nimble/pull/765#discussion_r389511349
 
 

 ##########
 File path: nimble/host/src/ble_hs_id.c
 ##########
 @@ -77,8 +87,6 @@ ble_hs_id_set_rnd(const uint8_t *rnd_addr)
 
     memcpy(ble_hs_id_rnd, rnd_addr, 6);
 
-    rc = 0;
 
 Review comment:
   this will left rc uninitialized in case of no error

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] sjanc merged pull request #765: nimble/host: Fix check for valid static random address

Posted by GitBox <gi...@apache.org>.
sjanc merged pull request #765: nimble/host: Fix check for valid static random address
URL: https://github.com/apache/mynewt-nimble/pull/765
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #765: nimble/host: Fix check for valid static random address

Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #765: nimble/host: Fix check for valid static random address
URL: https://github.com/apache/mynewt-nimble/pull/765#discussion_r389514883
 
 

 ##########
 File path: nimble/host/src/ble_hs_id.c
 ##########
 @@ -77,8 +87,6 @@ ble_hs_id_set_rnd(const uint8_t *rnd_addr)
 
     memcpy(ble_hs_id_rnd, rnd_addr, 6);
 
-    rc = 0;
 
 Review comment:
   In case of no error, rc will be set in [rc = ble_hs_hci_util_set_random_addr(rnd_addr);](https://github.com/apache/mynewt-nimble/pull/765/files#diff-1544015ed01ed7d5bbb3804be5fb1ba6R83)

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] utzig commented on issue #765: nimble/host: Fix check for valid static random address

Posted by GitBox <gi...@apache.org>.
utzig commented on issue #765: nimble/host: Fix check for valid static random address
URL: https://github.com/apache/mynewt-nimble/pull/765#issuecomment-595779199
 
 
   @prasad-alatkar Rebasing on latest `master` should fix the CI issues.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [mynewt-nimble] prasad-alatkar commented on a change in pull request #765: nimble/host: Fix check for valid static random address

Posted by GitBox <gi...@apache.org>.
prasad-alatkar commented on a change in pull request #765: nimble/host: Fix check for valid static random address
URL: https://github.com/apache/mynewt-nimble/pull/765#discussion_r388720848
 
 

 ##########
 File path: nimble/host/src/ble_hs_id.c
 ##########
 @@ -58,14 +58,20 @@ ble_hs_id_set_rnd(const uint8_t *rnd_addr)
 {
     uint8_t addr_type_byte;
     int rc;
-    uint8_t all_zeros[BLE_DEV_ADDR_LEN] = {0}, all_ones[BLE_DEV_ADDR_LEN] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
+    int i, rnd_part_sum = 0;
 
 
 Review comment:
   Sure @sjanc and yes I agree that your suggested changes (almost all) are more readable. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services