Handle UCS-2 data coding scheme for SMS messsages.

Handle UCS-2 data coding scheme for SMS messsages.

Also, handle alpha format for SMS sender "number". Turns out carriers
sometimes use this instead of E.164 when they originate a message.

BUG=chromium-os:13617
TEST=Unit tests using data taken from actual messages sent from
a Russian carrier to a Russian handset.

Change-Id: I78db5fff784f0d1dc0381e60768c177b58b6d800

R=njw@chromium.org

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=0c0da8f

[Nathan Williams, 2011-03-30 22:31:54]

http://codereview.chromium.org/6740005/diff/1/sms_message.cc
File sms_message.cc (right):

http://codereview.chromium.org/6740005/diff/1/sms_message.cc#newcode151
sms_message.cc:151: uint8_t dcs = pdu[tp_pid_offset+1] & 0x0c;
We're losing the detection of some unsupported cases here - seems like the mask
should be 0xec, instead.

http://codereview.chromium.org/6740005/diff/1/utilities.cc
File utilities.cc (right):

http://codereview.chromium.org/6740005/diff/1/utilities.cc#newcode279
utilities.cc:279: 
Is it possible for the SMS UCS2 strings to start with a byte-order mark?
Big-endian certainly seems to be conventional here, but I haven't seen anything
ruling out little-endian with a BOM, or an extraneous big-endian BOM.

http://codereview.chromium.org/6740005/diff/1/utilities.cc#newcode284
utilities.cc:284: } else if (0x80 <= ucs2char && ucs2char <= 0x7ff) {
The <= part is redundant, though probably harmless.

http://codereview.chromium.org/6740005/diff/1/utilities.cc#newcode305
utilities.cc:305: // First map each UTF-8 character to its GSM7 equivalent.
s/GSM7/UCS-2/, or ditch the comment entirely, since it's not really a multistep
process.

[Eric Shienbrood, 2011-03-31 20:03:59]

PTAL

http://codereview.chromium.org/6740005/diff/1/sms_message.cc
File sms_message.cc (right):

http://codereview.chromium.org/6740005/diff/1/sms_message.cc#newcode151
sms_message.cc:151: uint8_t dcs = pdu[tp_pid_offset+1] & 0x0c;
On 2011/03/30 22:31:54, Nathan Williams wrote:
> We're losing the detection of some unsupported cases here - seems like the
mask
> should be 0xec, instead.

Done.

http://codereview.chromium.org/6740005/diff/1/utilities.cc
File utilities.cc (right):

http://codereview.chromium.org/6740005/diff/1/utilities.cc#newcode279
utilities.cc:279: 
On 2011/03/30 22:31:54, Nathan Williams wrote:
> Is it possible for the SMS UCS2 strings to start with a byte-order mark?
> Big-endian certainly seems to be conventional here, but I haven't seen
anything
> ruling out little-endian with a BOM, or an extraneous big-endian BOM.

Given what the 3GPP spec 23.040 says, can we skip worrying about this for the
moment? I know it would be more robust to deal with it, but I'd rather wait
until we know there's a need.

http://codereview.chromium.org/6740005/diff/1/utilities.cc#newcode284
utilities.cc:284: } else if (0x80 <= ucs2char && ucs2char <= 0x7ff) {
On 2011/03/30 22:31:54, Nathan Williams wrote:
> The <= part is redundant, though probably harmless.

Why is it redundant?

http://codereview.chromium.org/6740005/diff/1/utilities.cc#newcode305
utilities.cc:305: // First map each UTF-8 character to its GSM7 equivalent.
On 2011/03/30 22:31:54, Nathan Williams wrote:
> s/GSM7/UCS-2/, or ditch the comment entirely, since it's not really a
multistep
> process.

Done.

[Nathan Williams, 2011-03-31 20:41:19]

Down to nits, so LGTM.

> On 2011/03/30 22:31:54, Nathan Williams wrote:
> > Is it possible for the SMS UCS2 strings to start with a byte-order mark?
> > Big-endian certainly seems to be conventional here, but I haven't seen
> anything
> > ruling out little-endian with a BOM, or an extraneous big-endian BOM.
> 
> Given what the 3GPP spec 23.040 says, can we skip worrying about this for the
> moment? I know it would be more robust to deal with it, but I'd rather wait
> until we know there's a need.

Yeah, given that explicit claim of big-endianness, and my total inability to
find evidence of little-endian UCS2 or UCS2 with BOMs in the wild in SMS
messages, skipping it seems fine. 

> 
> http://codereview.chromium.org/6740005/diff/1/utilities.cc#newcode284
> utilities.cc:284: } else if (0x80 <= ucs2char && ucs2char <= 0x7ff) {
> On 2011/03/30 22:31:54, Nathan Williams wrote:
> > The <= part is redundant, though probably harmless.
> 
> Why is it redundant?

Oops, I didn't see that both clauses were <=. I meant that the "0x80 <=
usc2char" part is redundant, since this is the else clause from "ucs2char <=
0x7f". Similarly, in the previous clause, "0 <= ucs2char" is redundant since
ucs2char is unsigned (actually, doesn't gcc warn about that?). But, still,
harmless and possibly good for clarity.

[Eric Shienbrood, 2011-03-31 21:06:21]

PTAL. I removed the unneeded comparisons.

On 2011/03/31 20:41:19, Nathan Williams wrote:
> Down to nits, so LGTM.
> 
> > On 2011/03/30 22:31:54, Nathan Williams wrote:
> > > Is it possible for the SMS UCS2 strings to start with a byte-order mark?
> > > Big-endian certainly seems to be conventional here, but I haven't seen
> > anything
> > > ruling out little-endian with a BOM, or an extraneous big-endian BOM.
> > 
> > Given what the 3GPP spec 23.040 says, can we skip worrying about this for
the
> > moment? I know it would be more robust to deal with it, but I'd rather wait
> > until we know there's a need.
> 
> Yeah, given that explicit claim of big-endianness, and my total inability to
> find evidence of little-endian UCS2 or UCS2 with BOMs in the wild in SMS
> messages, skipping it seems fine. 
> 
> > 
> > http://codereview.chromium.org/6740005/diff/1/utilities.cc#newcode284
> > utilities.cc:284: } else if (0x80 <= ucs2char && ucs2char <= 0x7ff) {
> > On 2011/03/30 22:31:54, Nathan Williams wrote:
> > > The <= part is redundant, though probably harmless.
> > 
> > Why is it redundant?
> 
> Oops, I didn't see that both clauses were <=. I meant that the "0x80 <=
> usc2char" part is redundant, since this is the else clause from "ucs2char <=
> 0x7f". Similarly, in the previous clause, "0 <= ucs2char" is redundant since
> ucs2char is unsigned (actually, doesn't gcc warn about that?). But, still,
> harmless and possibly good for clarity.