754777 - B2G RIL: Add a Helper to read String delimiters

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

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.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Patch to readStringDelimiter helper function

Comment 2

[Philipp von Weitershausen [:philikon]]

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`.

Comment 3

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: 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

Comment 4

[Philipp von Weitershausen [:philikon]]

(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!

Comment 5

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f03005c65982

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/f03005c65982