From 6e799d83dc73175584495144d2231d1f6a3dff82 Mon Sep 17 00:00:00 2001
From: Rob Bocchino <bocchino@jpl.nasa.gov>
Date: Tue, 18 Jul 2023 09:53:16 -0700
Subject: [PATCH] Clarify valid buffers (#2138)

* Clarify valid buffers

* Revise Buffer SDD
---
 Fw/Buffer/Buffer.cpp  |  4 ++++
 Fw/Buffer/Buffer.hpp  |  4 ++++
 Fw/Buffer/docs/sdd.md | 26 ++++++++++++++++++--------
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/Fw/Buffer/Buffer.cpp b/Fw/Buffer/Buffer.cpp
index d244279c97..65ff479bfb 100644
--- a/Fw/Buffer/Buffer.cpp
+++ b/Fw/Buffer/Buffer.cpp
@@ -61,6 +61,10 @@ bool Buffer::operator==(const Buffer& src) const {
     return (this->m_bufferData == src.m_bufferData) && (this->m_size == src.m_size) && (this->m_context == src.m_context);
 }
 
+bool Buffer::isValid() const {
+    return (this->m_bufferData != nullptr) && (this->m_size > 0);
+}
+
 U8* Buffer::getData() const {
     return this->m_bufferData;
 }
diff --git a/Fw/Buffer/Buffer.hpp b/Fw/Buffer/Buffer.hpp
index 50d021216b..f24e7971a5 100644
--- a/Fw/Buffer/Buffer.hpp
+++ b/Fw/Buffer/Buffer.hpp
@@ -118,6 +118,10 @@ class Buffer : public Fw::Serializable {
     // Accessor functions
     // ----------------------------------------------------------------------
 
+    //! Returns true if the buffer is valid (data pointer != nullptr and size > 0)
+    //!
+    bool isValid() const;
+
     //! Returns wrapped data pointer
     //!
     U8* getData() const;
diff --git a/Fw/Buffer/docs/sdd.md b/Fw/Buffer/docs/sdd.md
index 4dba905ea1..dddc1394a6 100644
--- a/Fw/Buffer/docs/sdd.md
+++ b/Fw/Buffer/docs/sdd.md
@@ -6,7 +6,7 @@
 This module provides the following elements:
 
 * A type `Fw::Buffer` representing a wrapper around a variable-size buffer. This allows for passing a reference to the
-allocated memory around without a copy. Typically the memory is allocated in a buffer manager or similar component but 
+allocated memory around without a copy. Typically the memory is allocated in a buffer manager or similar component but
 this is not required.
 * A port `Fw::BufferGet` for requesting a buffer of type `Fw::Buffer` from
 a [`BufferManager`](../../../Svc/BufferManager/docs/sdd.md) and similar components.
@@ -30,8 +30,14 @@ Name | Type | Accessors | Purpose
 `m_context`    | `U32` | `getContext()`/`setContext()` | Context of buffer's origin. Used to track buffers created by [`BufferManager`](../../../Svc/BufferManager/docs/sdd.md)
 `m_serialize_repr` | `Fw::ExternalSerializeBuffer` | `getSerializeRepr()` | Interface for serialization to internal buffer
 
-If the size of the data is set to 0, the pointer returned by `getData()` and the serialization interface returned by
-`getSerializeRepr()` are considered invalid and should not be used.
+A value _B_ of type `Fw::Buffer` is **valid** if `m_bufferData != nullptr` and
+`m_size > 0`; otherwise it is **invalid**.
+The interface function `isValid` reports whether a buffer is valid.
+Calling this function on a buffer _B_ returns `true` if _B_ is valid, otherwise `false`.
+
+If a buffer _B_ is invalid, then the pointer returned by _B_ `.getData()` and the
+serialization interface returned by
+_B_ `.getSerializeRepr()` are considered invalid and should not be used.
 
 The `getSerializeRepr()` function may be used to interact with the wrapped data buffer by serializing types to and from
 the data region.
@@ -40,7 +46,7 @@ the data region.
 ### 2.2 The Port Fw::BufferGet
 
 As shown in the following diagram, `Fw::BufferGet` has one argument `size` of type `U32`. It returns a value of type
-`Fw::Buffer`. The returned `Fw::Buffer`'s size must be checked for validity before using.
+`Fw::Buffer`. The returned `Fw::Buffer` must be checked for validity before using.
 
 ![`Fw::BufferGet` Diagram](img/BufferGetBDD.jpg "Fw::BufferGet Port")
 
@@ -53,16 +59,20 @@ As shown in the following diagram, `Fw::BufferSend` has one argument `fwBuffer`
 ## 3 Usage Notes
 
 Components allocating `Fw::Buffer` objects may use the `m_context` field at their discretion. This field is typically
-used to track the origin of the buffer for eventual allocation. When a component fails to allocate memory, it must set
-the `m_size` field to zero to indicate that the buffer is invalid.
+used to track the origin of the buffer for eventual allocation.
+
+When a component fails to allocate memory, it must set
+the `m_bufferData` field to `nullptr` and/or set the `m_size` field to zero to indicate that the buffer is invalid.
 
-Receivers of `Fw::Buffer` objects are expected to check the `m_size` field before using the buffer.
+A receiver of an `Fw::Buffer` object _B_ must check that _B_ is valid before accessing the
+data stored in _B_.
+To check validity, you can call the interface function `isValid()`.
 
 ### Serializing and Deserializing with `Fw::Buffer`
 
 Users can obtain a SerializeBuffer, `sb`, by calling `getSerializeRepr()`. This serialize buffer is backed by the memory
 of the `Fw::Buffer` and is initially empty.  Users can serialize and deserialize through `sb` to copy to/from the backed
-memory. 
+memory.
 
 The state of `sb` persists as long as the current `Fw::Buffer` object exists as it is stored as a member. However, all
 `Fw::Buffer` constructors initialize `sb` to an empty state including the `Fw::Buffer` copy constructor. Thus, if an