[b2g-bluetooth] nsTArray should not be passed back as return value

RESOLVED FIXED in mozilla18

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: qdot, Assigned: ericchou)

Tracking

Trunk
mozilla18
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [LOE:S])

Attachments

(1 attachment, 1 obsolete attachment)

From Bug 756299:

---

::: dom/bluetooth/BluetoothService.h
@@ +196,4 @@
>                  const nsAString& aDeviceAddress,
>                  nsAString& aDevicePath) = 0;
>  
> +  virtual nsTArray<PRUint32>

I know this wasn't introduced in this patch, but don't we normally try to avoid returning nsTArrays by value to avoid extra copies?

---

AddReservedServices should take a nsTArray<PRUint32>&, not return a new one.
(Assignee)

Updated

6 years ago
Assignee: nobody → echou
(Assignee)

Updated

6 years ago
Whiteboard: [LOE:S]
(Assignee)

Comment 1

6 years ago
Created attachment 658889 [details] [diff] [review]
v1: pass a service handle container from caller instead of returning a new nsTArraye
Attachment #658889 - Flags: review?(kyle)
(Assignee)

Comment 2

6 years ago
also fixed the inconsistency of using PRUint32 and uint32_t
Comment on attachment 658889 [details] [diff] [review]
v1: pass a service handle container from caller instead of returning a new nsTArraye

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

::: dom/bluetooth/linux/BluetoothDBusService.cpp
@@ +1478,2 @@
>  BluetoothDBusService::AddReservedServicesInternal(const nsAString& aAdapterPath,
> +                                                  const nsTArray<uint32_t>& aServices, 

Nit: extra space at end of line.
Attachment #658889 - Flags: review?(kyle) → review+
(Assignee)

Comment 4

6 years ago
Created attachment 659151 [details] [diff] [review]
Final: pass a service handle container from caller instead of returning a new nsTArray, r=qdot

Nit picked.
Attachment #658889 - Attachment is obsolete: true

Comment 7

6 years ago
https://hg.mozilla.org/mozilla-central/rev/ab24958a514d
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.