B2G RIL: Add a Helper to read String delimiters

RESOLVED FIXED in mozilla15

Status

()

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

People

(Reporter: allstars, Assigned: allstars)

Tracking

Trunk
mozilla15
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

In ril_worker.js,
there are many code snippets like

let delimiter = Buf.readUint16();
if (!(length & 1)) {
  delimiter |= Buf.readUint16();
}
if (DEBUG) {
  if (delimiter != 0) {
    debug("Something's wrong, found string delimiter: " + delimiter);
  }
}


Since it can be simplified as a function,
we should add a helper for this.
Assignee: nobody → yhuang
Created attachment 623601 [details] [diff] [review]
Patch to readStringDelimiter helper function
Attachment #623601 - Flags: review?(philipp)
Comment on attachment 623601 [details] [diff] [review]
Patch to readStringDelimiter helper function

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

Yay! But having this be a method on `GsmPDUHelper` doesn't make much sense. Let's put those methods on `Buf`.
Attachment #623601 - Flags: review?(philipp) → feedback+
Created attachment 623914 [details] [diff] [review]
Patch to readStringDelimiter helper function , V2

Hi, Philikon
Thanks for that comment, yes, it makes much sense to add this helper in Buf.

Do I need to add r=philikon in this patch?

thanks
Attachment #623601 - Attachment is obsolete: true
Attachment #623914 - Flags: review?(philipp)
Attachment #623914 - Flags: review?(philipp) → review+
(In reply to Yoshi Huang[:yoshi] from comment #3)
> Do I need to add r=philikon in this patch?

In the future this would be helpful, yes. It saves me having to add it manually.

Thanks for the updated patch!
https://hg.mozilla.org/integration/mozilla-inbound/rev/f03005c65982
Target Milestone: --- → mozilla15
https://hg.mozilla.org/mozilla-central/rev/f03005c65982
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.