From fe626b14fd772bc7af01f2210f11712863036bf3 Mon Sep 17 00:00:00 2001 From: William Date: Wed, 11 Sep 2024 13:06:01 +0100 Subject: [PATCH] encourage the use of CopyCharSpanToMutableCharSpanWithTruncation (#35444) * Updated the service area documentation to encourage the use of CopyCharSpanToMutableCharSpanWithTruncation for memory safety. Updated the rvc-app example to use this method. * Restyled by whitespace --------- Co-authored-by: Restyled.io --- .../rvc-app/rvc-common/src/rvc-device.cpp | 6 ++--- .../src/rvc-service-area-delegate.cpp | 6 ++--- .../service-area-delegate.h | 23 +++++++++++-------- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/examples/rvc-app/rvc-common/src/rvc-device.cpp b/examples/rvc-app/rvc-common/src/rvc-device.cpp index 88b8095ddf8bdd..02a75c51f56074 100644 --- a/examples/rvc-app/rvc-common/src/rvc-device.cpp +++ b/examples/rvc-app/rvc-common/src/rvc-device.cpp @@ -168,7 +168,7 @@ bool RvcDevice::SaIsSetSelectedAreasAllowed(MutableCharSpan & statusText) { if (mOperationalStateInstance.GetCurrentOperationalState() == to_underlying(OperationalState::OperationalStateEnum::kRunning)) { - CopyCharSpanToMutableCharSpan("cannot set the Selected Areas while the device is running"_span, statusText); + CopyCharSpanToMutableCharSpanWithTruncation("cannot set the Selected Areas while the device is running"_span, statusText); return false; } return true; @@ -179,14 +179,14 @@ bool RvcDevice::SaHandleSkipArea(uint32_t skippedArea, MutableCharSpan & skipSta if (mServiceAreaInstance.GetCurrentArea() != skippedArea) { // This device only supports skipping the current location. - CopyCharSpanToMutableCharSpan("the skipped area does not match the current area"_span, skipStatusText); + CopyCharSpanToMutableCharSpanWithTruncation("the skipped area does not match the current area"_span, skipStatusText); return false; } if (mOperationalStateInstance.GetCurrentOperationalState() != to_underlying(OperationalState::OperationalStateEnum::kRunning)) { // This device only accepts the skip are command while in the running state - CopyCharSpanToMutableCharSpan("skip area is only accepted when the device is running"_span, skipStatusText); + CopyCharSpanToMutableCharSpanWithTruncation("skip area is only accepted when the device is running"_span, skipStatusText); return false; } diff --git a/examples/rvc-app/rvc-common/src/rvc-service-area-delegate.cpp b/examples/rvc-app/rvc-common/src/rvc-service-area-delegate.cpp index b46206ec50ee75..1a6ba6d8c95999 100644 --- a/examples/rvc-app/rvc-common/src/rvc-service-area-delegate.cpp +++ b/examples/rvc-app/rvc-common/src/rvc-service-area-delegate.cpp @@ -134,7 +134,7 @@ bool RvcServiceAreaDelegate::IsValidSelectAreasSet(const Span & if (!GetInstance()->GetSupportedAreaById(selectedAreas[0], ignoredIndex, tempArea)) { areaStatus = SelectAreasStatus::kUnsupportedArea; - CopyCharSpanToMutableCharSpan("unable to find selected area in supported areas"_span, statusText); + CopyCharSpanToMutableCharSpanWithTruncation("unable to find selected area in supported areas"_span, statusText); return false; } @@ -145,14 +145,14 @@ bool RvcServiceAreaDelegate::IsValidSelectAreasSet(const Span & if (!GetInstance()->GetSupportedAreaById(areaId, ignoredIndex, tempArea)) { areaStatus = SelectAreasStatus::kUnsupportedArea; - CopyCharSpanToMutableCharSpan("unable to find selected area in supported areas"_span, statusText); + CopyCharSpanToMutableCharSpanWithTruncation("unable to find selected area in supported areas"_span, statusText); return false; } if (tempArea.mapID.Value() != mapId) { areaStatus = SelectAreasStatus::kInvalidSet; - CopyCharSpanToMutableCharSpan("all selected areas must be in the same map"_span, statusText); + CopyCharSpanToMutableCharSpanWithTruncation("all selected areas must be in the same map"_span, statusText); return false; } } diff --git a/src/app/clusters/service-area-server/service-area-delegate.h b/src/app/clusters/service-area-server/service-area-delegate.h index 3983e8d6ed7c25..f82f71fea31d13 100644 --- a/src/app/clusters/service-area-server/service-area-delegate.h +++ b/src/app/clusters/service-area-server/service-area-delegate.h @@ -55,9 +55,10 @@ class Delegate * @brief Can the selected locations be set by the client in the current operating mode? * @param[out] statusText text describing why the selected locations cannot be set (if return is false). * Max size kMaxSizeStatusText. - * Note: If the return is false and statusText is not successfully set, for example due to a string that can be longer than - * kMaxSizeStatusText, the size of this value should be set to 0 with .reduce_size(0) to avoid callers using un-initialized - * memory. + * Note: statusText must be successfully set if the return is false. Use CopyCharSpanToMutableCharSpanWithTruncation to + * ensure that a message is copied successfully. Otherwise, ensure that if setting the statusText can fail (e.g., due + * to exceeding kMaxSizeStatusText) the size of this value is set to 0 with .reduce_size(0) to avoid callers using + * un-initialized memory. * @return true if the current device state allows selected locations to be set by user. * * @note The statusText field SHOULD indicate why the request is not allowed, given the current mode @@ -77,9 +78,10 @@ class Delegate * @param[in] selectedAreas List of new selected locations. * @param[out] locationStatus Success if all checks pass, error code if failure. * @param[out] statusText text describing failure (see description above). Max size kMaxSizeStatusText. - * Note: If the return is false and statusText is not successfully set, for example due to a string that can be longer than - * kMaxSizeStatusText, the size of this value should be set to 0 with .reduce_size(0) to avoid callers using un-initialized - * memory. + * Note: statusText must be successfully set if the return is false. Use CopyCharSpanToMutableCharSpanWithTruncation to + * ensure that a message is copied successfully. Otherwise, ensure that if setting the statusText can fail (e.g., due + * to exceeding kMaxSizeStatusText) the size of this value is set to 0 with .reduce_size(0) to avoid callers using + * un-initialized memory. * @return true if success. * * @note If the SelectAreas command is allowed when the device is operating and the selected locations change to none, the @@ -93,9 +95,10 @@ class Delegate * calling this method. * @param[in] skippedArea the area ID to skip. * @param[out] skipStatusText text describing why the current location cannot be skipped. Max size kMaxSizeStatusText. - * Note: If the return is false and skipStatusText is not successfully set, for example due to a string that can be longer than - * kMaxSizeStatusText, the size of this value should be set to 0 with .reduce_size(0) to avoid callers using un-initialized - * memory. + * Note: skipStatusText must be successfully set if the return is false. Use CopyCharSpanToMutableCharSpanWithTruncation to + * ensure that a message is copied successfully. Otherwise, ensure that if setting the skipStatusText can fail (e.g., due + * to exceeding kMaxSizeStatusText) the size of this value is set to 0 with .reduce_size(0) to avoid callers using + * un-initialized memory. * @return true if command is successful, false if the received skip request cannot be handled due to the current mode of the * device. * @@ -120,7 +123,7 @@ class Delegate virtual bool HandleSkipArea(uint32_t skippedArea, MutableCharSpan & skipStatusText) { // device support of this command is optional - CopyCharSpanToMutableCharSpan("Skip Current Area command not supported by device"_span, skipStatusText); + CopyCharSpanToMutableCharSpanWithTruncation("Skip Current Area command not supported by device"_span, skipStatusText); return false; }