A Practical Checklist for Code Submissions #419
Replies: 1 comment
-
Change Request: Code Documentation & Style Compliance AuditDate: 2026-02-24 1. Reference StandardThis change request is based on a checklist derived from GNU Coding Standards
2. Executive Summary97 project source files were audited. Key findings:
3. File-Level Documentation Violations (F1, F2)F1 - Missing Purpose StatementAll files include the SPDX copyright/license header, but the following
Internal Headers (
|
| Header | Proposed Purpose Statement |
|---|---|
Instance.hpp |
Declares the Instance class, which represents a single MXL SDK session and owns the FlowManager, FlowIoFactory, DomainWatcher, and all open flow readers, writers, and synchronization groups. |
FlowManager.hpp |
Declares the FlowManager class, documenting its CRUD responsibilities (create, open, close, delete, list) over flow filesystem resources within a single MXL domain directory. |
FlowParser.hpp |
Declares the FlowParser class, which parses an NMOS IS-04 Flow resource JSON string and exposes typed accessors for ID, format, grain rate, payload size, channel count, and other flow properties. |
FlowData.hpp |
Declares the abstract FlowData base class, which wraps the primary SharedMemoryInstance<Flow> mapping and provides constexpr accessors to the Flow, mxlFlowInfo, and FlowState structures within it. |
FlowInfo.hpp |
Declares the operator<< overload for mxlFlowInfo, enabling human-readable stream formatting of a flow's configuration and runtime information. |
FlowOptionsParser.hpp |
Declares the FlowOptionsParser class, which parses a JSON options blob into typed optional fields (maxCommitBatchSizeHint, maxSyncBatchSizeHint) with a generic get<T>() accessor for extension fields. |
FlowWriter.hpp |
Declares the abstract FlowWriter base class with the common writer interface (getId, getFlowData, getFlowInfo, getFlowConfigInfo, getFlowRuntimeInfo, isExclusive), implemented by all concrete writer subclasses. |
FlowWriterFactory.hpp |
Declares the FlowWriterFactory abstract interface with pure-virtual factory methods for creating DiscreteFlowWriter and ContinuousFlowWriter instances. |
FlowReader.hpp |
Declares the abstract FlowReader base class with the common reader interface (getId, getDomain, getFlowData, getFlowInfo, getFlowConfigInfo, getFlowRuntimeInfo), implemented by all concrete reader subclasses. |
FlowReaderFactory.hpp |
Declares the FlowReaderFactory abstract interface with pure-virtual factory methods for creating DiscreteFlowReader and ContinuousFlowReader instances. |
FlowIoFactory.hpp |
Declares the combined FlowIoFactory abstract class, which inherits both FlowReaderFactory and FlowWriterFactory and adds type-erased createFlowReader/createFlowWriter dispatch methods. |
FlowSynchronizationGroup.hpp |
Declares the FlowSynchronizationGroup class, which holds a list of flow reader references and provides a waitForDataAt method to synchronize on a common origin time across multiple flows. |
FlowState.hpp |
Declares the FlowState struct stored in shared memory alongside mxlFlowInfo, holding the file inode (for stale-mapping detection) and a 32-bit syncCounter used as the futex word for writer-to-reader notifications. |
Flow.hpp |
Declares the internal Flow struct (combining mxlFlowInfo and FlowState), the GrainHeader/Grain structs (with 8 KiB-aligned payload offset), and layout constants for shared memory representation of flows and grains. |
DiscreteFlowData.hpp |
Declares the DiscreteFlowData class, which extends FlowData with a vector of per-grain SharedMemoryInstance<Grain> mappings and accessors for grain count, emplace, and indexed grain access. |
DiscreteFlowReader.hpp |
Declares the abstract DiscreteFlowReader interface with waitForGrain, blocking and non-blocking getGrain, and isFlowValid methods for consuming frame-by-frame discrete flow data. |
DiscreteFlowWriter.hpp |
Declares the abstract DiscreteFlowWriter interface with getGrainInfo, openGrain, commit, and cancel methods for producing discrete grain data into a flow. |
ContinuousFlowData.hpp |
Declares the ContinuousFlowData class, which extends FlowData with a supplemental SharedMemorySegment for the multi-channel ring-buffer and constexpr accessors for channel count, sample word size, buffer length, and channel data pointer. |
ContinuousFlowReader.hpp |
Declares the abstract ContinuousFlowReader interface with waitForSamples, blocking and non-blocking getSamples, and isFlowValid methods for consuming continuous sample-stream flow data via wrapped multi-buffer slices. |
ContinuousFlowWriter.hpp |
Declares the abstract ContinuousFlowWriter interface with openSamples, commit, and cancel methods for producing continuous multi-channel sample data into a ring-buffer flow. |
PosixFlowIoFactory.hpp |
Declares the PosixFlowIoFactory concrete factory struct, which implements all four FlowReaderFactory/FlowWriterFactory virtual methods to produce POSIX-specific reader and writer objects. |
SharedMemory.hpp |
Declares SharedMemoryBase (RAII mmap wrapper with access mode, lock mode, size, and validity) and the typed SharedMemoryInstance<T> and SharedMemorySegment templates for pointer-typed access to mapped regions. |
DomainWatcher.hpp |
Declares DomainWatcher, which runs a background thread using inotify/kqueue to watch flow access files in the domain directory and updates lastReadTime on the associated discrete flow writers. |
Logging.hpp |
Declares the MXL_TRACE–MXL_ERROR logging macros, wrapping spdlog calls in exception-safe blocks with compile-time level filtering based on NDEBUG. |
PathUtils.hpp |
Declares the canonical filesystem path constants and all makeFlow*/makeGrain*/makeChannel* helper functions used to construct well-known file paths within an MXL flow directory. |
MediaUtils.hpp |
Declares getV210LineLength and get10BitAlphaLineLength, utility functions for computing the padded byte stride of packed video formats used to size grain payloads. |
Thread.hpp |
Declares the this_thread namespace functions yield, yieldProcessor, sleep, and sleepUntil, providing platform-portable thread scheduling and clock-aware sleep primitives. |
Time.hpp |
Declares comparison operators for timespec, the Clock enumeration, the Timepoint and Duration opaque types, and arithmetic/conversion functions. |
Timing.hpp |
Declares currentTime(Clock) and currentTimeUTC() and by inclusion exposes the full Timepoint/Duration/Clock type system to consumers. |
Sync.hpp |
Declares the templated waitUntilChanged, wakeOne, and wakeAll primitives for cross-process futex-based synchronization on 32-bit aligned shared memory words. |
IndexConversion.hpp |
Declares constexpr inline timestampToIndex and indexToTimestamp, converting between nanosecond TAI timestamps and integer grain/sample indices at a given rational edit rate using 128-bit arithmetic. |
Rational.hpp |
Declares isValid and ==/!= comparison operators for mxlRational, providing basic arithmetic validation and equality semantics for rational-number edit/grain rates. |
detail/ClockHelpers.hpp |
Declares clockToId (mapping Clock enum to clockid_t) and getClockOffset (returning the 37-second TAI leap-second offset on platforms lacking CLOCK_TAI). |
OFI Fabrics Internal Files (lib/fabrics/ofi/src/internal/) — Proposed Purpose Statements
| Header | Proposed Purpose Statement |
|---|---|
Address.hpp |
Declares FabricAddress, a wrapper around a raw libfabric endpoint address supporting serialization to/from Base64 strings and retrieval from an open fid_t. |
AddressVector.hpp |
Declares the AddressVector RAII class wrapping a libfabric fi_av, mapping human-readable endpoint addresses to fabric-level address tokens for data-transfer operations. |
Base64.hpp |
Provides a header-only Base64 encode/decode implementation used to serialize binary fabric addresses for out-of-band exchange between initiator and target processes. |
Completion.hpp |
Declares the Completion class representing a single entry from a libfabric completion queue, with Data, Error, and Truncated variant types conveying transfer outcome details. |
CompletionQueue.hpp |
Declares the CompletionQueue RAII class wrapping a libfabric fi_cq, with methods to read and wait for completions with configurable timeout and queue size. |
Domain.hpp |
Declares the Domain RAII class wrapping a libfabric fi_domain, owning memory-region registrations and providing factory methods to create domains from FabricInfo. |
Endpoint.hpp |
Declares the Endpoint RAII class wrapping a libfabric fi_ep, providing methods for connecting, sending/receiving, and performing RMA write operations over a fabric. |
Event.hpp |
Declares the Event class with ConnectionRequested, Connected, and Shutdown variant types for connection lifecycle management. |
EventQueue.hpp |
Declares the EventQueue RAII class wrapping a libfabric fi_eq, with methods to read and wait for connection events. |
Exception.hpp |
Declares the Exception class carrying a message and mxlStatus error code, and mxlStatusFromFiErrno for translating libfabric error codes. |
Fabric.hpp |
Declares the Fabric RAII class wrapping a libfabric fid_fabric top-level object, with a static open factory that selects a fabric matching a given FabricInfoView. |
FabricInfo.hpp |
Declares FabricInfo (owning RAII wrapper around fi_info), FabricInfoView (non-owning view), and FabricInfoList (linked-list from fi_getinfo), providing accessors for provider, endpoint type, and capabilities. |
FabricVersion.hpp |
Declares fiVersion(), returning the libfabric API version this library was compiled against for version negotiation during fi_getinfo queries. |
Format.hpp |
Declares fmt::formatter specializations for mxlFabricsProvider and ofi::Provider, enabling direct use in fmt::format calls for diagnostic logging. |
LocalRegion.hpp |
Declares LocalRegion (source memory descriptor with address, length, and desc pointer) and LocalRegionGroup (scatter-gather list) used as the source side of fabric data transfers. |
MemoryRegion.hpp |
Declares the MemoryRegion RAII class wrapping a libfabric fid_mr, with a static reg factory that registers a Region with a Domain under specified access flags. |
Provider.hpp |
Declares the Provider enumeration (TCP, VERBS, EFA, SHM) and conversion functions between the internal enum, the public API enum, and provider name strings. |
Region.hpp |
Declares the Region class representing an unregistered memory region (with address, length, and Location — host or CUDA device), input to MemoryRegion::reg. |
RegisteredRegion.hpp |
Declares RegisteredRegion, combining a MemoryRegion and its originating Region to provide toRemote() and toLocal() conversion methods for transfer descriptors. |
RemoteRegion.hpp |
Declares RemoteRegion (remote RMA target descriptor with virtual address, length, and remote key) and RemoteRegionGroup (scatter-gather list) for RDMA write operations. |
VariantUtils.hpp |
Declares the overloaded<Ts...> variadic template helper enabling the visitor pattern with inline lambdas over std::variant types used throughout the fabrics layer. |
Files that PASS (have purpose/design documentation in comments):
lib/include/mxl/mxl.h- documents domain/tmpfs design, GC strategylib/include/mxl/flow.h- documents grain lifecycle, invalidationlib/include/mxl/flowinfo.h- documents flow configuration semanticslib/include/mxl/time.h- documents SMPTE ST 2059 epoch basislib/include/mxl/dataformat.h- documents NMOS IS-04 compliance
F2 - Missing Assumptions & Constraints
Files with platform-specific behaviour that lack explicit constraint
documentation at the top of the file:
| File | Implicit Constraint | Documented? |
|---|---|---|
lib/internal/src/Sync.cpp |
Linux futex / macOS os_sync | No |
lib/internal/src/Thread.cpp |
x86 vs ARM pause, clock_nanosleep | No |
lib/internal/src/SharedMemory.cpp |
POSIX shm, posix_fallocate vs ftruncate | No |
lib/internal/src/DomainWatcher.cpp |
inotify+epoll (Linux) / kqueue (macOS) | No |
lib/internal/src/FlowManager.cpp |
renameat2 (Linux) / renamex_np (macOS) | No |
lib/src/flow.cpp |
flock() POSIX advisory locks | No |
4. Function-Level Documentation Violations (FN1-FN4)
Public API (lib/include/mxl/*.h) - COMPLIANT
The public headers provide excellent Doxygen documentation with:
- Verb-first descriptions (FN1): Yes
\param[in],\param[out]annotations (FN2): Yes\returnwith error code meanings (FN3): Yes\notefor side effects where relevant (FN4): Partial
Internal Implementation Files - NON-COMPLIANT
The .cpp implementation files rely entirely on the corresponding header
for documentation. While the public API headers are thorough, the
internal classes and free functions are largely undocumented.
FN4 (Thread-safety) is universally absent. No file documents:
- Which locks must be held before calling a function
- Whether a function is reentrant or thread-safe
- What global state is mutated
This is critical for a shared-memory IPC library.
Below is a per-file, per-function catalog with suggested Doxygen comments.
lib/internal/src/Instance.cpp (14 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | (anon)::initializeLogging() |
/// Initialize the spdlog console logger and apply MXL_LOG_LEVEL environment variable overrides. Called once via std::call_once. |
| 2 | (anon)::parseOptionsJson(options, config) |
/// Parse a JSON string into a picojson object. \param[in] options JSON string. \param[out] config Parsed object. \return true on success, false on parse error or non-object root. |
| 3 | Instance::Instance(mxlDomain, options, flowIoFactory, watcher) |
/// Construct an Instance for the given domain directory. Initializes logging (once), parses options, and creates the FlowManager, FlowIoFactory, and DomainWatcher. \param[in] mxlDomain Canonical path to the MXL domain directory (must exist). \param[in] options JSON options string (may be empty). \param[in] flowIoFactory Ownership-transferred factory for creating flow readers/writers. \param[in] watcher Shared DomainWatcher for reader-activity detection. \note Thread-safety: Not thread-safe; construct on a single thread before sharing. |
| 4 | Instance::~Instance() |
/// Destroy the Instance, stopping the DomainWatcher and cleaning up leaked flow writers by deleting their underlying flows. \note Thread-safety: Must not be called concurrently with any other Instance method. |
| 5 | Instance::getFlowReader(flowId) |
/// Obtain or create a FlowReader for the given UUID string. If a reader already exists it is reference-counted. \param[in] flowId UUID string of the flow. \return Raw pointer to the reader (owned by the Instance). \throws std::runtime_error on invalid UUID or missing flow. \note Thread-safety: Acquires _mutex; safe for concurrent use. |
| 6 | Instance::releaseReader(reader) |
/// Release a previously obtained FlowReader. Decrements the reference count and destroys the reader when it reaches zero, also removing it from any synchronization groups. \param[in] reader Pointer obtained from getFlowReader(); nullptr is a no-op. \note Thread-safety: Acquires _mutex; safe for concurrent use. |
| 7 | Instance::releaseWriter(writer) |
/// Release a previously obtained FlowWriter. Decrements the reference count; on last release, attempts to acquire an exclusive lock and deletes the flow from the filesystem. \param[in] writer Pointer obtained from createFlowWriter(); nullptr is a no-op. \note Thread-safety: Acquires _mutex; safe for concurrent use. |
| 8 | Instance::createFlowWriter(flowDef, options) |
/// Create or open a FlowWriter for the flow described by a JSON NMOS Flow resource. \param[in] flowDef NMOS IS-04 Flow JSON string. \param[in] options Optional JSON options string. \return Tuple of {config info, writer pointer, true if newly created}. \throws std::runtime_error on invalid format. \note Thread-safety: Acquires _mutex; safe for concurrent use. |
| 9 | Instance::createOrOpenDiscreteFlowData(flowDef, parser, optionsParser) |
/// Create shared-memory resources for a discrete flow or open an existing one. Computes grain count from the configured history duration. \return Pair of {FlowData, true if newly created}. \note Thread-safety: Called under _mutex by createFlowWriter. |
| 10 | Instance::createOrOpenContinuousFlowData(flowDef, parser, optionsParser) |
/// Create shared-memory resources for a continuous flow or open an existing one. Computes buffer length from the configured history duration, page-aligned. \return Pair of {FlowData, true if newly created}. \note Thread-safety: Called under _mutex by createFlowWriter. |
| 11 | Instance::garbageCollect() |
/// Remove flow directories whose data file can be exclusively locked (i.e., no active writer). Best-effort: never throws. \return Number of flows successfully removed. \note Thread-safety: Safe for concurrent use (filesystem-level locking). |
| 12 | Instance::parseOptions(options) |
/// Parse the domain-level options.json and instance-level options string to extract configuration values (currently only history_duration). \param[in] options Instance-level JSON options string. \note Thread-safety: Called only during construction. |
| 13 | Instance::createFlowSynchronizationGroup() |
/// Allocate a new FlowSynchronizationGroup and return a pointer to it. The group is owned by the Instance. \return Pointer to the newly created group. \note Thread-safety: NOT thread-safe; caller must synchronize. |
| 14 | Instance::releaseFlowSynchronizationGroup(group) |
/// Remove and destroy a FlowSynchronizationGroup previously created by createFlowSynchronizationGroup(). \param[in] group Pointer to the group to release. \note Thread-safety: NOT thread-safe; caller must synchronize. |
lib/internal/src/FlowManager.cpp (12 functions, 2 partially documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | (anon)::createTemporaryFlowDirectory(base) |
(Already documented — PASS) |
| 2 | (anon)::publishFlowDirectory(source, dest) |
/// Atomically rename a temporary flow directory to its final public UUID-named path using renameat2 (Linux) or renamex_np (macOS) with NOREPLACE semantics. \param[in] source Temporary directory path. \param[in] dest Target UUID-named directory path. \return true if renamed successfully, false if dest already exists. \throws std::system_error on unexpected rename failure. |
| 3 | (anon)::sanitizeFlowFormat(format) |
(Already documented — PASS) |
| 4 | (anon)::writeFlowDescriptor(flowDir, flowDef) |
/// Write the NMOS IS-04 Flow JSON resource definition to the flow_def.json file inside the given flow directory. \param[in] flowDir Path to the flow directory. \param[in] flowDef JSON string to write. \throws std::filesystem::filesystem_error on I/O failure. |
| 5 | (anon)::initCommonFlowConfigInfo(...) |
/// Initialize an mxlCommonFlowConfigInfo struct from the given flow parameters. Copies the UUID, sets format, grain rate, batch hints, and defaults payload location to host memory. \return Populated mxlCommonFlowConfigInfo. |
| 6 | (anon)::initFlowRuntimeInfo() |
/// Initialize an mxlFlowRuntimeInfo with both lastWriteTime and lastReadTime set to the current TAI time. \return Populated mxlFlowRuntimeInfo. |
| 7 | (anon)::initFlowState(flowDataPath) |
/// Initialize a FlowState by stat'ing the flow data file to capture its inode (used for stale-mapping detection). \param[in] flowDataPath Path to the flow data file. \return Populated FlowState. \throws std::filesystem::filesystem_error if stat fails. |
| 8 | FlowManager::FlowManager(in_mxlDomain) |
/// Construct a FlowManager for the given domain directory. Canonicalizes the path and validates it exists. \param[in] in_mxlDomain Path to the MXL domain directory. \throws std::filesystem::filesystem_error if path does not exist or is not a directory. |
| 9 | FlowManager::createOrOpenDiscreteFlow(...) |
/// Create a discrete flow's filesystem resources (data file, grain files, access file, flow_def.json) atomically in a temp directory, then publish via atomic rename. If the UUID already exists, open the existing flow instead. \return Pair of {true if newly created, DiscreteFlowData}. \throws std::runtime_error on format mismatch or I/O failure. \note Side-effect: creates files and directories under the domain root. |
| 10 | FlowManager::createOrOpenContinuousFlow(...) |
/// Create a continuous flow's filesystem resources (data file, channel buffers, flow_def.json) atomically in a temp directory, then publish via atomic rename. If the UUID already exists, open the existing flow instead. \return Pair of {true if newly created, ContinuousFlowData}. \throws std::runtime_error on format mismatch or I/O failure. \note Side-effect: creates files and directories under the domain root. |
| 11 | FlowManager::openFlow(in_flowId, in_mode) |
/// Open an existing flow by UUID. Determines the flow format from the shared-memory header and delegates to openDiscreteFlow or openContinuousFlow. \param[in] in_flowId UUID of the flow. \param[in] in_mode Access mode (READ_ONLY or READ_WRITE; CREATE_READ_WRITE is invalid here). \return Polymorphic FlowData pointer. \throws std::invalid_argument on unsupported version or invalid access mode. |
| 12 | FlowManager::openDiscreteFlow(flowDir, sharedFlowInstance) |
/// Open all grain shared-memory files for an existing discrete flow. \param[in] flowDir Path to the published flow directory. \param[in] sharedFlowInstance Pre-opened flow header mapping. \return DiscreteFlowData with all grains mapped. |
| 13 | FlowManager::openContinuousFlow(flowDir, sharedFlowInstance) |
/// Open the channel buffer shared-memory file for an existing continuous flow. \param[in] flowDir Path to the published flow directory. \param[in] sharedFlowInstance Pre-opened flow header mapping. \return ContinuousFlowData with channel buffers mapped. |
| 14 | FlowManager::deleteFlow(flowData) |
/// Extract the flow UUID from a FlowData, close the FlowData, then delete the flow directory. \param[in] flowData Ownership-transferred FlowData to close and delete. \return true if the directory was removed. \note Effectively noexcept — converts exceptions to false. |
| 15 | FlowManager::deleteFlow(flowId) |
/// Delete the flow directory for the given UUID. \param[in] flowId UUID of the flow to delete. \return true if the directory was removed. \note Effectively noexcept — converts exceptions to false. |
| 16 | FlowManager::listFlows() |
/// Enumerate all flow UUIDs in the domain directory by scanning for directories matching the *.mxl-flow pattern. \return Vector of valid UUIDs found. \throws std::filesystem::filesystem_error if the domain directory does not exist. |
| 17 | FlowManager::getFlowDef(flowId) |
/// Read and return the NMOS IS-04 Flow JSON definition from the flow_def.json file for the given UUID. \param[in] flowId UUID of the flow. \return JSON string content. \throws std::filesystem::filesystem_error if the file is not found. |
| 18 | FlowManager::getDomain() |
/// Return the canonical domain directory path. \return const reference to the domain path. |
lib/internal/src/FlowParser.cpp (10 functions, 4 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | (anon)::translateFlowFormat(format) |
(Already documented — PASS) |
| 2 | (anon)::testField(in_obj, in_field) |
(Already documented — PASS) |
| 3 | (anon)::fetchAs<T>(in_object, in_field) |
(Already documented — PASS) |
| 4 | (anon)::extractRational(in_object) |
/// Extract an mxlRational from a JSON object containing "numerator" and optional "denominator" fields. Normalizes by GCD. \param[in] in_object JSON object with rational number fields. \return Normalized mxlRational. \throws std::invalid_argument if "numerator" is missing. |
| 5 | (anon)::validateGroupHint(in_object) |
/// Validate the NMOS group hint tag (urn:x-nmos:tag:grouphint/v1.0) format: "<group-name>:<role>[:<scope>]". \param[in] in_object Root JSON object of the flow resource. \return true if the group hint is present and all entries are valid. \note Logs errors on validation failure. |
| 6 | FlowParser::FlowParser(in_flowDef) |
/// Parse an NMOS IS-04 Flow resource JSON string, extracting and validating the flow ID, format, grain/sample rate, label, group hint, and (for video) frame dimensions and interlace mode. For interlaced video, doubles the grain rate to express a field rate. \param[in] in_flowDef JSON string of the NMOS Flow resource. \throws std::invalid_argument on malformed JSON or missing/invalid fields. \throws std::domain_error on unsupported flow format. |
| 7 | FlowParser::getPayloadSize() |
/// Compute the total payload size in bytes for a single grain/sample based on the flow's format, media type, and dimensions. Handles v210, v210a (fill+key), smpte291 data, and audio bit depths. \return Payload size in bytes. \throws std::invalid_argument on unsupported media type or invalid dimensions. |
| 8 | FlowParser::getPayloadSliceLengths() |
/// Compute the per-slice byte lengths for each plane of a grain. For video/v210 this is one line stride; for video/v210a it returns fill and key strides in planes 0 and 1; for data it is 1 byte. \return Array of up to MXL_MAX_PLANES_PER_GRAIN slice lengths. \throws std::invalid_argument for unsupported formats. |
| 9 | FlowParser::getTotalPayloadSlices() |
/// Compute the total number of slices per grain. For video this is the number of lines (halved for interlaced); for data this is DATA_FORMAT_GRAIN_SIZE. \return Total slice count. \throws std::invalid_argument for unsupported formats. |
| 10 | FlowParser::getChannelCount() |
/// Return the channel_count from the JSON flow definition, defaulting to 1 if absent. \return Channel count (>=1). \throws std::invalid_argument if the field is present but not a positive integer. |
lib/internal/src/PosixDiscreteFlowReader.cpp (10 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | (anon)::updateFileAccessTime(fd) |
/// Update the access timestamp of the file descriptor to the current time using futimens, leaving the modification time unchanged. \param[in] fd Open file descriptor. \return true on success. |
| 2 | PosixDiscreteFlowReader::PosixDiscreteFlowReader(manager, flowId, data) |
/// Construct a discrete flow reader. Opens the flow's access file for timestamp updates (failure is tolerated for read-only volumes). \param[in] manager FlowManager providing the domain path. \param[in] flowId UUID of the flow. \param[in] data Ownership-transferred DiscreteFlowData with all grains mapped. |
| 3 | PosixDiscreteFlowReader::waitForGrain(in_index, in_minValidSlices, in_deadline) |
/// Block until the grain at in_index has at least in_minValidSlices valid slices, or until in_deadline. Checks for stale flow on timeout. \return MXL_STATUS_OK, MXL_ERR_OUT_OF_RANGE_TOO_EARLY/TOO_LATE, or MXL_ERR_FLOW_INVALID. \note Thread-safety: Safe for concurrent use (reads shared memory atomically via futex). |
| 4 | PosixDiscreteFlowReader::getGrain(in_index, in_minValidSlices, in_deadline, out_grainInfo, out_payload) |
/// Block until the grain is available, then return its metadata and a pointer to the payload. Updates the access file timestamp on success. Checks for stale flow on timeout. \param[out] out_grainInfo Grain metadata. \param[out] out_payload Pointer into shared memory for the grain payload. \return mxlStatus code. \note Thread-safety: Safe for concurrent use. |
| 5 | PosixDiscreteFlowReader::getGrain(in_index, in_minValidSlices, out_grainInfo, out_payload) |
/// Non-blocking variant: return the grain immediately if available. Updates the access file timestamp on success. Checks for stale flow if too early. \return mxlStatus code. |
| 6 | PosixDiscreteFlowReader::getGrainImpl(in_index, in_minValidSlices, out_grainInfo, out_payload) |
/// Core non-blocking grain lookup. Checks the ring buffer head index and valid slice count. \return MXL_STATUS_OK, MXL_ERR_OUT_OF_RANGE_TOO_EARLY, or MXL_ERR_OUT_OF_RANGE_TOO_LATE. |
| 7 | PosixDiscreteFlowReader::getGrainImpl(in_index, in_minValidSlices, in_deadline, out_grainInfo, out_payload) |
/// Blocking grain lookup using futex-based waitUntilChanged on the flow's syncCounter. Retries until the grain is available, the request is too late, or the deadline expires. \return mxlStatus code. |
| 8 | PosixDiscreteFlowReader::isFlowValid() |
/// Check whether the flow's shared-memory mapping is still valid (not stale). \return true if valid. \note Thread-safety: Safe for concurrent use. |
| 9 | PosixDiscreteFlowReader::isFlowValidImpl() |
/// Compare the inode stored in FlowState against the current inode of the flow data file on disk. A mismatch indicates the flow was deleted and recreated. \return true if the inodes match. |
lib/internal/src/PosixDiscreteFlowWriter.cpp (10 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | PosixDiscreteFlowWriter::PosixDiscreteFlowWriter(manager, flowId, data, watcher) |
/// Construct a discrete flow writer, registering with the DomainWatcher for reader activity notifications. \param[in] watcher Shared DomainWatcher; this writer is registered for inotify/kqueue events on its access file. |
| 2 | PosixDiscreteFlowWriter::~PosixDiscreteFlowWriter() |
/// Destroy the writer, unregistering from the DomainWatcher. Suppresses exceptions in the destructor. |
| 3 | PosixDiscreteFlowWriter::getGrainInfo(in_index) |
/// Return a copy of the mxlGrainInfo for the grain at the given absolute index. \param[in] in_index Absolute grain index (modulo grain count for ring-buffer offset). \return Copy of the grain's info struct. |
| 4 | PosixDiscreteFlowWriter::openGrain(in_index, out_grainInfo, out_payload) |
/// Open a grain slot for writing at the given absolute index. Returns the current grain metadata and a pointer to the writable payload buffer. Sets _currentIndex for the subsequent commit. \param[in] in_index Absolute grain index. \param[out] out_grainInfo Current grain info. \param[out] out_payload Writable pointer to the grain's payload in shared memory. \return MXL_STATUS_OK or MXL_ERR_UNKNOWN if no flow data. |
| 5 | PosixDiscreteFlowWriter::commit(mxlGrainInfo) |
/// Commit the grain opened by openGrain(). Updates the ring-buffer head index, copies the grain info, records the write timestamp, increments the syncCounter, and wakes all waiting readers via futex. \param[in] mxlGrainInfo Updated grain metadata (index must match _currentIndex). \return MXL_STATUS_OK, MXL_ERR_INVALID_ARG on index mismatch, or MXL_ERR_UNKNOWN. \note Thread-safety: Writes to shared memory; only one writer should commit at a time. |
| 6 | PosixDiscreteFlowWriter::cancel() |
/// Cancel the currently opened grain without committing. Resets _currentIndex to MXL_UNDEFINED_INDEX. \return MXL_STATUS_OK. |
| 7 | PosixDiscreteFlowWriter::isExclusive() |
/// Check whether this writer holds an exclusive lock on the flow data file. \return true if the lock is exclusive. |
| 8 | PosixDiscreteFlowWriter::makeExclusive() |
/// Attempt to upgrade the shared lock to an exclusive lock (non-blocking). \return true if upgraded successfully, false if another process holds a shared lock. |
lib/internal/src/PosixContinuousFlowReader.cpp (8 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | PosixContinuousFlowReader::PosixContinuousFlowReader(...) |
/// Construct a continuous flow reader, caching the channel count and buffer length from the ContinuousFlowData. |
| 2 | PosixContinuousFlowReader::waitForSamples(index, deadline) |
/// Block until samples up to index are available, or until deadline expires. Returns MXL_ERR_FLOW_INVALID if the flow is stale on timeout. \note Thread-safety: Safe for concurrent use (futex-based wait). |
| 3 | PosixContinuousFlowReader::getSamples(index, count, deadline, payloadBuffersSlices) |
/// Blocking variant: wait for samples, then compute wrap-around fragment pointers into the ring buffer. \param[out] payloadBuffersSlices Multi-buffer slice descriptors. \return mxlStatus code. |
| 4 | PosixContinuousFlowReader::getSamples(index, count, payloadBuffersSlices) |
/// Non-blocking variant: return samples immediately if available. \return mxlStatus code. |
| 5 | PosixContinuousFlowReader::isFlowValid() |
/// Check whether the flow's shared-memory mapping is still valid. \return true if valid. |
| 6 | PosixContinuousFlowReader::isFlowValidImpl() |
/// Compare stored inode against on-disk inode for stale-flow detection. \return true if inodes match. |
| 7 | PosixContinuousFlowReader::getSamplesImpl(index, count, payloadBuffersSlices) |
/// Core non-blocking sample lookup. Computes wrap-around offsets in the ring buffer and fills the fragment pointers. Readers may access up to half the buffer behind the head. \return MXL_STATUS_OK, MXL_ERR_OUT_OF_RANGE_TOO_EARLY, or MXL_ERR_OUT_OF_RANGE_TOO_LATE. |
| 8 | PosixContinuousFlowReader::getSamplesImpl(index, count, deadline, payloadBuffersSlices) |
/// Blocking sample lookup using futex-based waitUntilChanged on the syncCounter. Retries until available or deadline expires. \return mxlStatus code. |
lib/internal/src/PosixContinuousFlowWriter.cpp (8 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | PosixContinuousFlowWriter::PosixContinuousFlowWriter(...) |
/// Construct a continuous flow writer, caching channel count, buffer length, and computing the sync batch thresholds from the flow config's maxCommitBatchSizeHint and maxSyncBatchSizeHint. |
| 2 | PosixContinuousFlowWriter::openSamples(index, count, payloadBufferSlices) |
/// Open a writable window of count samples ending at index in the ring buffer. Computes wrap-around fragment pointers and strides. \param[in] count Must not exceed half the buffer length. \param[out] payloadBufferSlices Mutable multi-buffer slice descriptors. \return MXL_STATUS_OK, MXL_ERR_INVALID_ARG if count is too large, or MXL_ERR_UNKNOWN. |
| 3 | PosixContinuousFlowWriter::commit() |
/// Commit the samples opened by openSamples(). Advances the head index and, if a sync batch boundary is crossed, increments the syncCounter and wakes all waiting readers via futex. \return MXL_STATUS_OK or MXL_ERR_UNKNOWN. \note Thread-safety: Only one writer should commit at a time. |
| 4 | PosixContinuousFlowWriter::cancel() |
/// Cancel the currently opened sample window without committing. \return MXL_STATUS_OK. |
| 5 | PosixContinuousFlowWriter::signalCompletedBatch() |
/// Determine whether the current commit crosses a sync batch boundary, warranting a futex wake to readers. Uses _earlySyncThreshold to signal before overshooting on the next commit. \return true if readers should be woken. \note Called only from commit(). |
| 6 | PosixContinuousFlowWriter::isExclusive() |
/// Check whether this writer holds an exclusive lock on the flow data file. \return true if exclusive. \throws std::runtime_error if flow data is not initialized. |
| 7 | PosixContinuousFlowWriter::makeExclusive() |
/// Attempt to upgrade the shared lock to an exclusive lock. \return true on success. \throws std::runtime_error if flow data is not initialized. |
lib/internal/src/SharedMemory.cpp (4 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | SharedMemoryBase::SharedMemoryBase(path, mode, payloadSize, lockMode) |
/// Open or create a file-backed shared-memory segment. On CREATE_READ_WRITE, creates the file exclusively and allocates payloadSize bytes using posix_fallocate (Linux) or ftruncate (macOS). Acquires an advisory flock lock according to lockMode, then mmaps the file. \param[in] path Filesystem path for the backing file. \param[in] mode Access mode (CREATE_READ_WRITE, READ_WRITE, or READ_ONLY). \param[in] payloadSize Minimum required file size in bytes (0 for open-existing modes). \param[in] lockMode None, Shared, or Exclusive. \throws std::system_error on open/fallocate/mmap/flock failure. \note Thread-safety: Construction is not thread-safe for the same path. |
| 2 | SharedMemoryBase::~SharedMemoryBase() |
/// Unmap the shared-memory region, release the advisory lock, and close the file descriptor. |
| 3 | SharedMemoryBase::touch() |
/// Update the file's access (and optionally modification) timestamp to the current time via futimens. \note Logs an error on failure but does not throw. |
| 4 | SharedMemoryBase::makeExclusive() |
/// Attempt a non-blocking upgrade from shared to exclusive flock. \return true if the exclusive lock was acquired, false if another holder prevents it. \throws std::runtime_error on bad file descriptor; std::system_error on unexpected flock error. |
lib/internal/src/DomainWatcher.cpp (9 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | DomainWatcher::DomainWatcher(in_domain) |
/// Construct a DomainWatcher for the given domain directory. Creates the platform-specific notification mechanism (inotify+epoll on Linux, kqueue on macOS) and starts the background processEvents thread. \param[in] in_domain Path to the domain directory (must be a directory). \throws std::filesystem::filesystem_error, std::system_error on setup failure. |
| 2 | DomainWatcher::~DomainWatcher() |
/// Stop the background thread and close all platform-specific file descriptors and watches. |
| 3 | DomainWatcher::count(id) |
/// Return the number of active watches for the given flow UUID. \note Thread-safety: Acquires _mutex. |
| 4 | DomainWatcher::size() |
/// Return the total number of active watches. \note Thread-safety: Acquires _mutex. |
| 5 | DomainWatcher::addFlow(writer, id) |
/// Register a discrete flow writer for reader-activity notifications. Adds an inotify/kqueue watch on the flow's access file if one doesn't already exist, and opens a read-write FlowData mapping for updating lastReadTime. \param[in] writer The writer to associate with the watch. \param[in] id UUID of the flow. \throws std::system_error if the watch cannot be added. \note Thread-safety: Acquires _mutex. |
| 6 | DomainWatcher::removeFlow(writer, id) |
/// Unregister a discrete flow writer. Removes the inotify/kqueue watch if no other writers are interested in the same flow. \note Thread-safety: Acquires _mutex. |
| 7 | DomainWatcher::processEvents() |
/// Background thread entry point. Polls for filesystem events (epoll_wait on Linux, kevent on macOS) with a 250ms timeout and dispatches to processEventBuffer/processPendingEvents. Runs until _running is set to false. |
| 8 | DomainWatcher::setWatch() (macOS) |
/// Rebuild the kevent monitoring list from the current _watches map. \note Thread-safety: Acquires _mutex. Called from processEvents on macOS. |
| 9 | DomainWatcher::processPendingEvents(numEvents) (macOS) / processEventBuffer(buffer, count) (Linux) |
/// Update FlowRuntimeInfo.lastReadTime to the current TAI time for each flow whose access file triggered a filesystem event. \note Thread-safety: Acquires _mutex. Called from processEvents. |
lib/internal/src/FlowSynchronizationGroup.cpp (5 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | ListEntry::ListEntry(reader, variant) |
/// Construct a list entry from a generic FlowReader, caching the grain rate from the flow's config info. \param[in] reader Reference to a FlowReader (discrete or continuous). \param[in] variant Discriminator for the reader type. |
| 2 | FlowSynchronizationGroup::addReader(DiscreteFlowReader, minValidSlices) |
/// Add a discrete flow reader to the synchronization group, or update minValidSlices if it is already a member. \param[in] reader The discrete reader to add. \param[in] minValidSlices Minimum valid slices required when waiting. |
| 3 | FlowSynchronizationGroup::addReader(ContinuousFlowReader) |
/// Add a continuous flow reader to the synchronization group. No-op if already a member. \param[in] reader The continuous reader to add. |
| 4 | FlowSynchronizationGroup::removeReader(reader) |
/// Remove a flow reader from the synchronization group. \param[in] reader The reader to remove. No-op if not found. |
| 5 | FlowSynchronizationGroup::waitForDataAt(originTime, deadline) |
/// Block until all readers in the group have data available at originTime, or deadline expires. For each reader, converts originTime to a grain/sample index and calls the appropriate wait method. Reorders readers by observed source delay to minimize blocking on subsequent calls. \param[in] originTime TAI timestamp to synchronize on. \param[in] deadline Maximum time to wait. \return MXL_STATUS_OK if all readers are ready, or the first error encountered. \note Thread-safety: Safe for concurrent use (delegates to reader wait methods). |
tools/mxl-info/main.cpp (8 functions, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | detail::isTerminal(os) |
/// Detect whether the given ostream is connected to a terminal (for color output decisions). \return true if the stream is stdout/stderr and isatty() returns non-zero. |
| 2 | detail::outputLatency(os, headIndex, grainRate, limit) |
/// Compute and print the grain/sample latency (current time index minus head index) with color coding: green if below limit, yellow at limit, red above. \param[in] limit Grain count (discrete) or buffer length (continuous). |
| 3 | detail::operator<<(os, mxlFlowInfo) |
/// Format an mxlFlowInfo as a multi-line diagnostic summary showing UUID, version, format, rates, batch sizes, runtime state, and format-specific fields. |
| 4 | detail::operator<<(os, LatencyPrinter) |
/// Format an mxlFlowInfo with an appended latency line. |
| 5 | getFlowDetails(flowDef) |
/// Extract the label and first group-hint tag value from a flow's JSON definition. \return Pair of {label, groupHint}, defaulting to "n/a" on parse failure. |
| 6 | listAllFlows(in_domain) |
/// Enumerate all flows in the domain, printing each as a CSV line: UUID, label, group-hint. \return EXIT_SUCCESS or EXIT_FAILURE. |
| 7 | printFlow(in_domain, in_id) |
/// Create a flow reader for the given UUID, print its formatted mxlFlowInfo with latency, and check whether the flow is active. \return EXIT_SUCCESS or EXIT_FAILURE. |
| 8 | garbageCollect(in_domain) |
/// Invoke mxlGarbageCollectFlows on the given domain and report success or failure. \return EXIT_SUCCESS or EXIT_FAILURE. |
tools/mxl-gst/sink.cpp (11 key functions/methods, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | GstreamerPipeline::start() |
/// Set the GStreamer pipeline to PLAYING and record the MXL TAI base time for PTS conversion. \throws std::runtime_error if pipeline is uninitialized. |
| 2 | GstreamerPipeline::pushBuffer(buffer, now) |
/// Push a GstBuffer to the appsrc element with PTS set relative to the MXL base time. \param[in] buffer Ownership not transferred. \param[in] now TAI timestamp for the buffer's PTS. |
| 3 | GstreamerPipeline::launchPipeline(pipelineDescription) |
/// Parse and launch a GStreamer pipeline from a description string, locate the "appsource" element, and configure a TAI system clock. \throws std::runtime_error on pipeline creation or clock setup failure. |
| 4 | MxlReader::MxlReader(domain, flowId) |
/// Create an MXL instance and flow reader for the given domain and flow UUID. \throws std::runtime_error if instance or reader creation fails. |
| 5 | MxlReader::run(VideoPipeline, readDelay) |
/// Main read loop for discrete video flows: requests grains at a computed read delay, blocks until available or deadline, pushes valid frames to GStreamer, and handles flow invalidation, too-early/too-late, and reconnection. |
| 6 | MxlReader::run(AudioPipeline, readDelay) |
/// Main read loop for continuous audio flows: requests sample windows at a computed read delay, blocks until available or deadline, copies non-interleaved samples to GStreamer audio buffers, and handles flow invalidation and reconnection. |
| 7 | MxlReader::handleInvalidFlow(requestedIndex) |
/// Handle flow invalidation by releasing the old reader and attempting to create a new one. \return true if successfully reconnected. |
| 8 | MxlReader::Cursor::Cursor(rate, windowSize, readDelay) |
/// Initialize a read cursor with the given rate and window size, computing the read delay in grains and aligning to the current time. |
| 9 | MxlReader::Cursor::next() |
/// Advance the cursor by one window, sleeping until the delivery deadline. |
| 10 | MxlReader::Cursor::realign(timeNow) |
/// Reset the cursor to the grain/sample index corresponding to the current time, re-applying the read delay. |
| 11 | readFlowDescriptor(domain, flowID) |
/// Create a temporary MXL instance to read and return the flow JSON definition for a given UUID. \throws std::runtime_error on failure. |
tools/mxl-gst/testsrc.cpp (8 key functions/methods, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | GstreamerPipeline::start() |
/// Set the pipeline to PLAYING and align the MXL base time to the next grain timestamp for epoch-aligned output. |
| 2 | GstreamerPipeline::pull(timeout) |
/// Pull a sample from the appsink with the given timeout. Converts the buffer's PTS to TAI time by adding the base time. \return GstreamerSampleRef (empty if no sample available). |
| 3 | GstreamerPipeline::launchPipeline(pipelineDescription, grainRate) |
/// Parse a GStreamer pipeline, locate the "appsink" element, and configure a TAI system clock. \throws std::runtime_error on failure. |
| 4 | MxlWriter::MxlWriter(domain, flow_descriptor, flowOptions) |
/// Create an MXL instance and flow writer for the given domain and flow descriptor JSON. \throws std::runtime_error if creation fails. |
| 5 | MxlWriter::run(VideoPipeline, offset) |
/// Main write loop for discrete video flows: pulls frames from GStreamer, detects skipped grains (committing them as invalid), copies pixel data in configurable slice batches with inter-batch sleeps, and commits. |
| 6 | MxlWriter::run(AudioPipeline, offset) |
/// Main write loop for continuous audio flows: pulls samples from GStreamer, detects skips (inserting silence), copies non-interleaved samples into the MXL ring buffer, and commits. |
| 7 | AudioPipeline::generateMixMatrix() |
/// Generate a GStreamer audioconvert mix-matrix string that maps each speaker channel to the corresponding input channel. \return Matrix string in GStreamer value format "<< (float)0, (float)1, ... >>". |
| 8 | readFile(filepath) |
/// Read the entire contents of a file into a string. \throws std::runtime_error on open failure. |
tools/mxl-gst/looping_filesrc.cpp (10 key functions/methods, 0 documented)
| # | Function | Suggested Comment |
|---|---|---|
| 1 | LoopingFilePlayer::cb_pad_added(element, pad, data) |
/// GStreamer decodebin "pad-added" callback. Dynamically creates and links video (queue→videorate→videoconvert→appsink) or audio (queue→audioconvert→appsink) sub-pipelines based on the pad's media type. |
| 2 | LoopingFilePlayer::LoopingFilePlayer(in_domain) |
/// Construct a player for the given MXL domain, creating the domain directory if needed and initializing an MXL instance. \throws std::runtime_error if instance creation fails. |
| 3 | LoopingFilePlayer::open(in_uri) |
/// Create a GStreamer pipeline with a looping_filesrc and decodebin, pause to negotiate caps, then create MXL flow writers for video and/or audio based on negotiated format information. \return true on success. |
| 4 | LoopingFilePlayer::start() |
/// Set the pipeline to PLAYING, align the base time to the next audio grain epoch, and start the video and audio processing threads. \return true on success. |
| 5 | LoopingFilePlayer::stop() |
/// Signal the video and audio threads to stop by setting running to false. |
| 6 | LoopingFilePlayer::createVideoFlowJson(...) |
/// Generate an NMOS IS-04 Flow JSON resource for v210 video with the given dimensions, frame rate, and colorimetry. \return UUID of the generated flow. |
| 7 | LoopingFilePlayer::createAudioFlowJson(...) |
/// Generate an NMOS IS-04 Flow JSON resource for audio with the given sample rate, channel count, and bit depth. \return UUID of the generated flow. |
| 8 | LoopingFilePlayer::getFlowOptions(maxCommitBatchSizeHint, maxSyncBatchSizeHint) |
/// Serialize flow options (commit/sync batch size hints) to a JSON string. |
| 9 | LoopingFilePlayer::videoThread() |
/// Background thread: pulls video samples from the GStreamer appsink, aligns timestamps with an MXL-to-GStreamer offset, writes grain data to the MXL flow writer, and generates invalid grains for skipped frames. |
| 10 | LoopingFilePlayer::audioThread() |
/// Background thread: pulls audio samples from the GStreamer appsink, aligns timestamps, writes non-interleaved sample data to the MXL continuous flow writer, and inserts silence for skipped samples. |
5. Inline Comment Violations (I1, I2)
I2 - Narrating-the-Obvious Comments Found
| File:Line | Comment | Issue |
|---|---|---|
lib/src/flow.cpp:100 |
+1 for the null terminator |
Obvious from strlen()+1 |
This is a minor issue overall. The project generally avoids this
anti-pattern.
I1 - Files Lacking "Why" Explanations
Files with complex logic but no explanatory inline comments:
| File | Concern |
|---|---|
lib/internal/src/FlowParser.cpp |
JSON parsing logic with magic constants (MAX_WIDTH=7680) but no rationale for limit values |
lib/internal/src/Instance.cpp |
Option parsing and initialization sequence with no explanation of ordering requirements |
tools/mxl-gst/*.cpp |
GStreamer integration with undocumented pipeline assumptions |
lib/fabrics/ofi/src/internal/Endpoint.cpp |
OFI endpoint lifecycle with no explanation of state transitions |
Files with GOOD inline "why" comments (exemplars):
lib/internal/src/Sync.cpp- EAGAIN semantics, platform differenceslib/internal/src/Thread.cpp- ARM memory barriers, nanosleep domainslib/internal/src/PosixContinuousFlowReader.cpp- stale flow detectionlib/internal/src/DomainWatcher.cpp- close-on-exec rationalelib/internal/src/FlowManager.cpp- atomic rename pattern
6. Typographical Issues Found
| File | Line | Issue |
|---|---|---|
lib/include/mxl/flowinfo.h |
~96 | "flo" should be "flow" |
7. Recommended Action Plan
Priority 1 - High Impact, Low Effort
-
Add file-level purpose comments to all
.cppand internal.hpp
files. A single sentence after the SPDX header is sufficient.
Example:// SPDX-FileCopyrightText: ... // SPDX-License-Identifier: ... // // Implements futex-based synchronization primitives for // inter-process coordination of shared-memory flow data.
-
Add platform constraint comments to the 6 platform-specific files
identified in Section 3 (F2). Example:// Requires: Linux (futex via SYS_futex) or macOS 11+ (os_sync_wait_on_address) // Thread-safety: All functions are safe for concurrent use.
-
Document thread-safety for all public and internal class methods
that touch shared memory or synchronization primitives.
Priority 2 - Medium Effort
-
Add Doxygen to internal headers (
lib/internal/include/).
The internal classes are the backbone of the library and should have
at least verb-first descriptions and parameter docs. -
Add inline "why" comments to files with complex logic,
particularly in the fabrics/OFI subsystem and GStreamer integration
tools. Use the exemplar files listed in Section 5 as a reference
for the expected level of commentary.
Priority 3 - Ongoing
-
Adopt the checklist (Section 1) as part of the code review
process for all future contributions. -
Fix the typo in
lib/include/mxl/flowinfo.hline ~96.
8. Summary Statistics
| Metric | Count |
|---|---|
| Total project source files audited | 97 |
| Files missing purpose statement (F1) | ~75 (77%) |
| Files missing constraint docs (F2) | 6 critical |
| Internal functions without docs (FN1-FN3) | ~120+ |
| Files with thread-safety docs (FN4) | 0 |
| Narrating-the-obvious comments (I2) | 1 |
| Files needing "why" comments (I1) | 4 flagged |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
A handy checklist before submitting any code to ensure it meets standard Linux and GNU documentation expectations
File Level
[ ] Top-of-file comment: Clearly state the file's purpose and its primary responsibilities. (GNU)
[ ] Clear assumptions and constraints: Document any specific platform requirements, ABI expectations, or required execution privileges. (GNU)
Function Level
[ ] Brief verb-first description: Start the function documentation with a clear action (e.g., "Allocates...", "Initializes...", "Parses..."). (GNU)
[ ] Parameters: Detail the precise meaning, valid ranges, and any special cases (such as NULL pointer handling) for all arguments. (GNU)
[ ] Returns: Explicitly define what constitutes success and explain the specific meanings of any returned error codes. (GNU)
[ ] Side effects and thread-safety: Note any global state changes, required locks, I/O operations, or reentrancy limitations. (Kernel)
Inline
[ ] Explain the "why": Focus your comments on non-obvious logic, workarounds, or hardware quirks rather than the mechanics of the code itself. (Kernel)
[ ] Avoid narrating obvious code: Do not write comments that simply repeat what the syntax inherently does (e.g., avoid i++; // increment i). (Kernel)
Formatting
[ ] ~80-column lines: Prefer keeping lines to 80 characters or fewer to maximize readability in standard Linux-style environments and terminal windows. (Kernel)
Beta Was this translation helpful? Give feedback.
All reactions