diff --git a/src/app/decoder.cpp b/src/app/decoder.cpp index 673cfb5a69295e..90ae94671ddb69 100644 --- a/src/app/decoder.cpp +++ b/src/app/decoder.cpp @@ -23,28 +23,13 @@ // InterPanHeader *interPanHeader) #include -#include +#include +#include +#include #include #include #include -template -struct Reader -{ -}; - -template <> -struct Reader<1> -{ - static uint8_t read(const uint8_t *& p) { return chip::Encoding::Read8(p); } -}; - -template <> -struct Reader<2> -{ - static uint16_t read(const uint8_t *& p) { return chip::Encoding::LittleEndian::Read16(p); } -}; - extern "C" { uint16_t extractApsFrame(uint8_t * buffer, uint16_t buf_length, EmberApsFrame * outApsFrame) @@ -55,39 +40,26 @@ uint16_t extractApsFrame(uint8_t * buffer, uint16_t buf_length, EmberApsFrame * return 0; } - const uint8_t * read_ptr = buffer; + chip::DataModelReader reader(buffer, buf_length); - // Skip first byte, because that's the always-0 frame control. - ++read_ptr; - --buf_length; - -#define TRY_READ(fieldName, fieldSize) \ - do \ - { \ - static_assert(sizeof(outApsFrame->fieldName) == fieldSize, "incorrect size for " #fieldName); \ - if (buf_length < fieldSize) \ - { \ - ChipLogError(Zcl, "Missing " #fieldName " when extracting APS frame"); \ - return 0; \ - } \ - outApsFrame->fieldName = Reader::read(read_ptr); \ - buf_length = static_cast(buf_length - fieldSize); \ - } while (0) - - TRY_READ(profileId, 2); - TRY_READ(clusterId, 2); - TRY_READ(sourceEndpoint, 1); - TRY_READ(destinationEndpoint, 1); - TRY_READ(options, 2); - TRY_READ(groupId, 2); - TRY_READ(sequence, 1); - TRY_READ(radius, 1); + CHIP_ERROR err = CHIP_NO_ERROR; -#undef TRY_READ - - // Cast is safe because buf_length is uint16_t, so we can't have moved too - // far along. - return static_cast(read_ptr - buffer); + // Skip first byte, because that's the always-0 frame control. + uint8_t ignored; + err = reader.ReadOctet(&ignored) + .Read16(&outApsFrame->profileId) + .ReadClusterId(&outApsFrame->clusterId) + .ReadEndpointId(&outApsFrame->sourceEndpoint) + .ReadEndpointId(&outApsFrame->destinationEndpoint) + .Read16(&outApsFrame->options) + .ReadGroupId(&outApsFrame->groupId) + .ReadOctet(&outApsFrame->sequence) + .ReadOctet(&outApsFrame->radius) + .StatusCode(); + SuccessOrExit(err); + +exit: + return err == CHIP_NO_ERROR ? reader.OctetsRead() : 0; } void printApsFrame(EmberApsFrame * frame) diff --git a/src/app/message-reader.h b/src/app/message-reader.h new file mode 100644 index 00000000000000..5f95c21ddb58dc --- /dev/null +++ b/src/app/message-reader.h @@ -0,0 +1,139 @@ +/* + * + * Copyright (c) 2020 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file + * Contains the API definition for a message buffer reader for the data + * model. This reader does the necessary bounds-checks before reading + * and updates its own state as the buffer is read. + */ + +#pragma once + +#include +#include +#include +#include +#include + +namespace chip { + +class DataModelReader +{ +public: + /** + * Create a data model reader from a given buffer and length. + * + * @param buffer The octet buffer to read from. The caller must ensure + * (most simply by allocating the reader on the stack) that + * the buffer outlives the reader. The buffer is allowed to + * be null if buf_len is 0. + * @param buf_len The number of octets in the buffer. + */ + DataModelReader(const uint8_t * buffer, uint16_t buf_len) : mReader(buffer, buf_len) {} + + /** + * Number of octets we have read so far. This might be able to go away once + * we do less switching back and forth between DataModelReader and raw + * buffers. + */ + uint16_t OctetsRead() const { return mReader.OctetsRead(); } + + /** + * The reader status. + */ + CHIP_ERROR StatusCode() const { return mReader.StatusCode(); } + + /** + * Read a cluster id. + * + * @param [out] cluster_id Where the cluster id goes. + * + * @return Whether the read succeeded. The read can fail if there are not + * enough octets available. + */ + CHECK_RETURN_VALUE DataModelReader & ReadClusterId(ClusterId * cluster_id) + { + mReader.RawRead(cluster_id); + return *this; + } + + /** + * Read an endpoint id. + * + * @param [out] endpoint_id Where the endpoint id goes. + * + * @return Whether the read succeeded. The read can fail if there are not + * enough octets available. + */ + CHECK_RETURN_VALUE DataModelReader & ReadEndpointId(EndpointId * endpoint_id) + { + mReader.RawRead(endpoint_id); + return *this; + } + + /** + * Read a group id. + * + * @param [out] group_id Where the group id goes. + * + * @return Whether the read succeeded. The read can fail if there are not + * enough octets available. + */ + CHECK_RETURN_VALUE DataModelReader & ReadGroupId(GroupId * group_id) + { + mReader.RawRead(group_id); + return *this; + } + + /** + * Read a single octet. + * + * @param [out] octet Where the octet goes. + * + * @return Whether the read succeeded. The read can fail if there are not + * enough octets available. + * + * @note Use of APIs that read some semantically-meaningful type is preferred. + */ + CHECK_RETURN_VALUE DataModelReader & ReadOctet(uint8_t * octet) + { + mReader.RawRead(octet); + return *this; + } + + /** + * Read a single 16-bit unsigned integer. + * + * @param [out] dest Where the 16-bit integer goes. + * + * @return Whether the read succeeded. The read can fail if there are not + * enough octets available. + * + * @note Use of APIs that read some semantically-meaningful type is preferred. + */ + CHECK_RETURN_VALUE DataModelReader & Read16(uint16_t * dest) + { + mReader.RawRead(dest); + return *this; + } + +private: + Encoding::LittleEndian::Reader mReader; +}; + +} // namespace chip diff --git a/src/lib/support/BufferReader.cpp b/src/lib/support/BufferReader.cpp index 3843f73c830f8c..97f3d72a74bc54 100644 --- a/src/lib/support/BufferReader.cpp +++ b/src/lib/support/BufferReader.cpp @@ -44,7 +44,7 @@ void ReadHelper(const uint8_t *& p, uint64_t * dest) } // anonymous namespace template -void Reader::Read(T * retval) +void Reader::RawRead(T * retval) { static_assert(CHAR_BIT == 8, "Our various sizeof checks rely on bytes and octets being the same thing"); @@ -63,10 +63,10 @@ void Reader::Read(T * retval) } // Explicit Read instantiations for the data types we want to support. -template void Reader::Read(uint8_t *); -template void Reader::Read(uint16_t *); -template void Reader::Read(uint32_t *); -template void Reader::Read(uint64_t *); +template void Reader::RawRead(uint8_t *); +template void Reader::RawRead(uint16_t *); +template void Reader::RawRead(uint32_t *); +template void Reader::RawRead(uint64_t *); } // namespace LittleEndian } // namespace Encoding diff --git a/src/lib/support/BufferReader.h b/src/lib/support/BufferReader.h index b6ada54ec1e5c4..ddf63dbe475c15 100644 --- a/src/lib/support/BufferReader.h +++ b/src/lib/support/BufferReader.h @@ -79,7 +79,7 @@ class Reader CHECK_RETURN_VALUE Reader & Read8(uint8_t * dest) { - Read(dest); + RawRead(dest); return *this; } @@ -96,7 +96,7 @@ class Reader CHECK_RETURN_VALUE Reader & Read16(uint16_t * dest) { - Read(dest); + RawRead(dest); return *this; } @@ -113,7 +113,7 @@ class Reader CHECK_RETURN_VALUE Reader & Read32(uint32_t * dest) { - Read(dest); + RawRead(dest); return *this; } @@ -130,17 +130,19 @@ class Reader CHECK_RETURN_VALUE Reader & Read64(uint64_t * dest) { - Read(dest); + RawRead(dest); return *this; } -protected: /** * Helper for our various APIs so we don't have to write out various logic - * multiple times. + * multiple times. This is public so that consumers that want to read into + * whatever size a logical thing they are reading into has don't have to + * hardcode the right API. This is meant for other reader classes that + * delegate to this one. */ template - void Read(T * retval); + void RawRead(T * retval); private: /**