944293 - [Bluetooth] Correct IS_PERIPHERAL COD and add error handling if COD is invalid

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

[Bluetooth] Correct IS_PERIPHERAL COD and add error handling if COD is invalid.
Peripheral bit mask shall be 0x0500 instead of 0xA. And we also need to handle if NS_ENSURE_TRUE_VOID(hasAudio || hasRendering || isPeripheral).

There will be SIGSEGV when mProfiles[mProfilesIndex].

Comment 1

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 944293 - [Bluetooth] Correct IS_PERIPHERAL COD and add error handling if COD is invalid

Comment 2

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 8339892 [details] [diff] [review]
Bug 944293 - [Bluetooth] Correct IS_PERIPHERAL COD and add error handling if COD is invalid

Review of attachment 8339892 [details] [diff] [review]:
-----------------------------------------------------------------

Please check my following comments.

::: dom/bluetooth/BluetoothProfileController.h
@@ +25,5 @@
>   * https://www.bluetooth.org/en-us/specification/assigned-numbers/baseband
>   */
>  
>  // Bit 23 ~ Bit 13: Major service class
> +#define GET_MAJOR_SERVICE_CLASS(cod) (cod & 0xffe000)

This macro is used to retrieve the value of major service class from CoD, so I think we should still keep the shift operations.

@@ +30,3 @@
>  
>  // Bit 12 ~ Bit 8: Major device class
> +#define GET_MAJOR_DEVICE_CLASS(cod)  (cod & 0x1f00)

ditto.

@@ +39,5 @@
>  
>  // Bit 18: Major service class = 0x20, Rendering
>  #define HAS_RENDERING(cod)           (cod & 0x40000)
>  
> +// Major device class = 0x0500, Peripheral

Well, the major device class should be 0x5 for Peripheral device.

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 944293 - [Bluetooth] Correct IS_PERIPHERAL COD and add error handling if COD is invalid

Comment 4

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 8340221 [details] [diff] [review]
Bug 944293 - [Bluetooth] Correct IS_PERIPHERAL COD and add error handling if COD is invalid

Review of attachment 8340221 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/BluetoothProfileController.h
@@ +40,5 @@
>  // Bit 18: Major service class = 0x20, Rendering
>  #define HAS_RENDERING(cod)           (cod & 0x40000)
>  
> +// Major device class = 0x5, Peripheral
> +#define IS_PERIPHERAL(cod)           (GET_MAJOR_DEVICE_CLASS(cod) & 0x5)

We'd like to ensure that the major device class is exactly 0x5.

For example, if the major device class is 0x4, the above macro returns 0x4, which means that the device is a peripheral device since (0x4  > 0). Does it make sense?

Comment 5

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 944293 - [Bluetooth] Correct IS_PERIPHERAL COD and add error handling if COD is invalid

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Sorry, I made a Monkey-like patch :(. You're right!

Comment 7

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 8340244 [details] [diff] [review]
Bug 944293 - [Bluetooth] Correct IS_PERIPHERAL COD and add error handling if COD is invalid

Review of attachment 8340244 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah!

Comment 8

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug 944293 - [Bluetooth] Correct IS_PERIPHERAL COD validation and add error handling if COD is invalid, r=gyeh

Comment 9

[Gina Yeh [:gyeh] [:ginayeh]]

https://hg.mozilla.org/integration/b2g-inbound/rev/66db2bb82088

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/66db2bb82088

Comment 11

[Joe Cheng [:jcheng] (please needinfo)]

triage: minus as this is related to HID which is not part of the feature