Skip to content

Commit

Permalink
Bug 1202386: Output clear HAL IPC errors, r=shuang
Browse files Browse the repository at this point in the history
This patch improves error logging in the IPC PDU packing and unpacking
code.
  • Loading branch information
tdz committed Sep 10, 2015
1 parent eaad9f6 commit 6340e04
Showing 1 changed file with 25 additions and 15 deletions.
40 changes: 25 additions & 15 deletions ipc/hal/DaemonSocketPDUHelpers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ Convert(bool aIn, uint8_t& aOut)
[false] = 0x00,
[true] = 0x01
};
if (NS_WARN_IF(aIn >= MOZ_ARRAY_LENGTH(sValue))) {
if (MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn >= MOZ_ARRAY_LENGTH(sValue), bool, uint8_t)) {
aOut = 0;
return NS_ERROR_ILLEGAL_VALUE;
}
Expand All @@ -96,8 +97,10 @@ Convert(bool aIn, int32_t& aOut)
nsresult
Convert(int aIn, uint8_t& aOut)
{
if (NS_WARN_IF(aIn < std::numeric_limits<uint8_t>::min()) ||
NS_WARN_IF(aIn > std::numeric_limits<uint8_t>::max())) {
if (MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn < std::numeric_limits<uint8_t>::min(), int, uint8_t) ||
MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn > std::numeric_limits<uint8_t>::max(), int, uint8_t)) {
aOut = 0; // silences compiler warning
return NS_ERROR_ILLEGAL_VALUE;
}
Expand All @@ -108,8 +111,10 @@ Convert(int aIn, uint8_t& aOut)
nsresult
Convert(int aIn, int16_t& aOut)
{
if (NS_WARN_IF(aIn < std::numeric_limits<int16_t>::min()) ||
NS_WARN_IF(aIn > std::numeric_limits<int16_t>::max())) {
if (MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn < std::numeric_limits<int16_t>::min(), int, int16_t) ||
MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn > std::numeric_limits<int16_t>::max(), int, int16_t)) {
aOut = 0; // silences compiler warning
return NS_ERROR_ILLEGAL_VALUE;
}
Expand All @@ -120,8 +125,10 @@ Convert(int aIn, int16_t& aOut)
nsresult
Convert(int aIn, int32_t& aOut)
{
if (NS_WARN_IF(aIn < std::numeric_limits<int32_t>::min()) ||
NS_WARN_IF(aIn > std::numeric_limits<int32_t>::max())) {
if (MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn < std::numeric_limits<int32_t>::min(), int, int32_t) ||
MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn > std::numeric_limits<int32_t>::max(), int, int32_t)) {
aOut = 0; // silences compiler warning
return NS_ERROR_ILLEGAL_VALUE;
}
Expand All @@ -136,7 +143,8 @@ Convert(uint8_t aIn, bool& aOut)
[0x00] = false,
[0x01] = true
};
if (NS_WARN_IF(aIn >= MOZ_ARRAY_LENGTH(sBool))) {
if (MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn >= MOZ_ARRAY_LENGTH(sBool), uint8_t, bool)) {
return NS_ERROR_ILLEGAL_VALUE;
}
aOut = sBool[aIn];
Expand Down Expand Up @@ -174,8 +182,10 @@ Convert(uint32_t aIn, int& aOut)
nsresult
Convert(uint32_t aIn, uint8_t& aOut)
{
if (NS_WARN_IF(aIn < std::numeric_limits<uint8_t>::min()) ||
NS_WARN_IF(aIn > std::numeric_limits<uint8_t>::max())) {
if (MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn < std::numeric_limits<uint8_t>::min(), uint32_t, uint8_t) ||
MOZ_HAL_IPC_CONVERT_WARN_IF(
aIn > std::numeric_limits<uint8_t>::max(), uint32_t, uint8_t)) {
aOut = 0; // silences compiler warning
return NS_ERROR_ILLEGAL_VALUE;
}
Expand All @@ -186,7 +196,7 @@ Convert(uint32_t aIn, uint8_t& aOut)
nsresult
Convert(size_t aIn, uint16_t& aOut)
{
if (NS_WARN_IF(aIn >= (1ul << 16))) {
if (MOZ_HAL_IPC_CONVERT_WARN_IF(aIn >= (1ul << 16), size_t, uint16_t)) {
aOut = 0; // silences compiler warning
return NS_ERROR_ILLEGAL_VALUE;
}
Expand Down Expand Up @@ -246,19 +256,19 @@ UnpackPDU(DaemonSocketPDU& aPDU, nsDependentCString& aOut)
// the string in the PDU, we can copy the actual bytes.

const char* str = reinterpret_cast<const char*>(aPDU.Consume(1));
if (NS_WARN_IF(!str)) {
if (MOZ_HAL_IPC_UNPACK_WARN_IF(!str, nsDependentCString)) {
return NS_ERROR_ILLEGAL_VALUE; // end of PDU
}

const char* end = static_cast<char*>(memchr(str, '\0', aPDU.GetSize() + 1));
if (NS_WARN_IF(!end)) {
if (MOZ_HAL_IPC_UNPACK_WARN_IF(!end, nsDependentCString)) {
return NS_ERROR_ILLEGAL_VALUE; // no string terminator
}

ptrdiff_t len = end - str;

const uint8_t* rest = aPDU.Consume(len);
if (NS_WARN_IF(!rest)) {
if (MOZ_HAL_IPC_UNPACK_WARN_IF(!rest, nsDependentCString)) {
// We couldn't consume bytes that should have been there.
return NS_ERROR_ILLEGAL_VALUE;
}
Expand Down Expand Up @@ -315,7 +325,7 @@ PDUInitOp::WarnAboutTrailingData() const
uint16_t payloadSize;
mPDU->GetHeader(service, opcode, payloadSize);

CHROMIUM_LOG(
detail::LogProtocolError(
"Unpacked PDU of type (%x,%x) still contains %zu Bytes of data.",
service, opcode, size);
}
Expand Down

0 comments on commit 6340e04

Please sign in to comment.