1113017 - [B2G][CBS] Additional message channel is set unexpectedly from ril_worker to ril daemon in GSM configuration.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Bevis Tseng[:bevis][:btseng](Exited)]

1. In ril_worker.js, an array of half-closed ranges: [[from, to), [from, to),
   ...] is used to collect all the message ids from CBMI/CBMIR/CBMID/MMI.
2. In ril.h, the data structure RIL_GSM_BroadcastSmsConfigInfo is adopted by 
   REQUEST_GSM_SET_BROADCAST_SMS_CONFIG:
   typedef struct {
       int fromServiceId;
       int toServiceId;
       int fromCodeScheme;
       int toCodeScheme;
       unsigned char selected;
   } RIL_GSM_BroadcastSmsConfigInfo;
   Where uFromServiceID - uToServiceID defines a range of CBM message identifiers.
   It seems not necessary to exclude the toServiceId from the range. 
3. However, things go wrong in setGsmSmsBroadcastConfig() in ril_worker.js:
  setGsmSmsBroadcastConfig: function(config) {
    let Buf = this.context.Buf;
    Buf.newParcel(REQUEST_GSM_SET_BROADCAST_SMS_CONFIG);

    let numConfigs = config ? config.length / 2 : 0;
    Buf.writeInt32(numConfigs);
    for (let i = 0; i < config.length;) {
      Buf.writeInt32(config[i++]);
      Buf.writeInt32(config[i++]); <- This is wrong! It shall be config[i++]-1.
      Buf.writeInt32(0x00);
      Buf.writeInt32(0xFF);
      Buf.writeInt32(1);
    }
    Buf.sendParcel();
 }

This bug is filed to fix the problem mentioned above to ensure the search list to be set properly.

Comment 1

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch v1: Update SearchList with correct ranges in GSM configuration. r=htsai.

Hi Hsinyi,

May I have your review for this quick fix?
(Modification for the corresponding test case is also included)

Thanks!

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8538383 [details] [diff] [review]
Patch v1: Update SearchList with correct ranges in GSM configuration. r=htsai.

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

::: dom/system/gonk/ril_worker.js
@@ +2154,2 @@
>        Buf.writeInt32(config[i++]);
> +      Buf.writeInt32(config[i++] - 1);

Nice catch, thank you.

Comment 3

[Bevis Tseng[:bevis][:btseng](Exited)]

update try server result:
https://tbpl.mozilla.org/?tree=Try&rev=cb4c0e7a536d

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/062566b84d83

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/062566b84d83

Comment 6

[Josh Cheng [:josh]]

Hi Bevis,
2.0M+ Thanks!

Comment 7

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Josh Cheng [:josh] from comment #6)
> Hi Bevis,
> 2.0M+ Thanks!

The same patch can be applied to v2.0M without conflict.

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

NI Kai-Zhen for 2.0m uplift.

Comment 10

[Kai-Zhen Li [:seinlin][:kli]]

https://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/10afe747b740

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Is this needed on v2.1 and/or v2.1s?

Comment 12

[Bevis Tseng[:bevis][:btseng](Exited)]

Comment on attachment 8538383 [details] [diff] [review]
Patch v1: Update SearchList with correct ranges in GSM configuration. r=htsai.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined:
1. Additional CBS Channel will be listened unexpectedly.
2. Cause configuration failed when enabling the CBS Channel 65534 as mentioned in duplicated bug 1121801.
Testing completed: Yes. Test Case included.
Risk to taking this patch (and alternatives if risky): No.
String or UUID changes made by this patch:N/A

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #11)
> Is this needed on v2.1 and/or v2.1s?

Thanks for reminding this! :)
I've requested the approval for uplifting to v2.1.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/5150abb65d20

Comment 15

[Vincent Liu[:vliu]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/5150abb65d20