Closed Bug 1206116 Opened 9 years ago Closed 9 years ago

bogus endianness changes in BluetoothMapSmsManager.cpp

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1166647

People

(Reporter: froydnj, Unassigned)

References

Details

BluetoothMapSmsManager.cpp has several cases of code that look like:

    case Map::AppParametersTagId::SubjectLength: {
      uint8_t subLength = *((uint8_t *)buf);
      // convert big endian to little endian
      subLength = (subLength >> 8) | (subLength << 8);
      BT_LOGR("msg subLength : %d", subLength);
      AppendNamedValue(aValues, "subLength", subLength);
      break;
    }

The problematic lines are:

      // convert big endian to little endian
      subLength = (subLength >> 8) | (subLength << 8);

First, because |subLength| is a uint8_t, there's no endian conversion to be done.  It's not obvious whether |subLength| was meant to be a 16-bit quantity, or whether somebody was too free with copying and pasting code.  Secondly, this endian conversion has the unfortunate effect of discarding |subLength|'s value--in effect, always reading zero.

There's also a similar:

    case Map::AppParametersTagId::ParameterMask: {
      // 4 bytes
      uint32_t parameterMask = *((uint32_t *)buf);
      // convert big endian to little endian
      parameterMask = (parameterMask >> 8) | (parameterMask << 8);
      BT_LOGR("msg parameterMask : %d", parameterMask);
      AppendNamedValue(aValues, "parameterMask", parameterMask);
      break;
    }

which doesn't actually endian-convert the 32-bit integer read; it actually scrambles the bytes up a bit.

It seems like both of these could cause some serious problems for bluetooth applications...
Yes, you're right. I will remove incorrect code in Bug 1166647.
Blocks: 1141954
Fix in bug 1166647.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.