Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor) Assignee
	Description • 11 years ago

[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].

	Gina Yeh [:gyeh] [:ginayeh]
	Comment 2 • 11 years ago

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.

	Gina Yeh [:gyeh] [:ginayeh]
	Comment 4 • 11 years ago

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?

	Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor) Assignee
	Comment 6 • 11 years ago

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

	Gina Yeh [:gyeh] [:ginayeh]
	Comment 7 • 11 years ago

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!

	Gina Yeh [:gyeh] [:ginayeh]
	Comment 9 • 11 years ago

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

	Carsten Book [:Tomcat]
	Comment 10 • 11 years ago

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

	Joe Cheng [:jcheng] (please needinfo)
	Comment 11 • 11 years ago

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