Define LOG_FILEPATH_SIZE constant for maintainability

- Add LOG_FILEPATH_SIZE constant (256) in both header and cpp
- Update LogEntry.filepath to use the constant
- Improve comments explaining width specifier relationship
- Addresses code review feedback about magic numbers

Co-authored-by: dakejahl <37091262+dakejahl@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2025-12-15 21:45:09 +00:00
co-authored by dakejahl
parent 8e0cabaeb7
commit 5037a96376
2 changed files with 14 additions and 5 deletions
+11 -4
View File
@@ -54,9 +54,13 @@ static const char *kLogDir = PX4_STORAGEDIR "/log";
#define PX4_MAX_FILEPATH PATH_MAX
#endif
// Size for log file path buffers - must match MavlinkLogHandler::LOG_FILEPATH_SIZE
#define LOG_FILEPATH_SIZE 256
#define LOG_FILEPATH_MAX_CHARS (LOG_FILEPATH_SIZE - 1) // Max chars that can be read (leave room for null terminator)
// Ensure PX4_MAX_FILEPATH is large enough for our buffer sizes
// LogEntry.filepath is 256 bytes, so PX4_MAX_FILEPATH must be at least 256
static_assert(PX4_MAX_FILEPATH >= 256, "PX4_MAX_FILEPATH must be at least 256 bytes for log file paths");
static_assert(PX4_MAX_FILEPATH >= LOG_FILEPATH_SIZE,
"PX4_MAX_FILEPATH must be at least LOG_FILEPATH_SIZE bytes for log file paths");
MavlinkLogHandler::MavlinkLogHandler(Mavlink &mavlink)
: _mavlink(mavlink)
@@ -178,8 +182,9 @@ void MavlinkLogHandler::state_listing()
char filepath[PX4_MAX_FILEPATH];
// If parsed lined successfully, send the entry
// Note: Using %255s to safely read into filepath buffer (max 255 chars + null terminator)
// This is conservative but safe across all platforms regardless of PX4_MAX_FILEPATH value
// Note: Using width specifier to prevent buffer overflow
// %255s reads max 255 chars + null terminator = 256 bytes (LOG_FILEPATH_SIZE)
// This is safe across all platforms (static_assert ensures PX4_MAX_FILEPATH >= LOG_FILEPATH_SIZE)
if (sscanf(line, "%" PRIu32 " %" PRIu32 " %255s", &time_utc, &size_bytes, filepath) != 3) {
PX4_DEBUG("sscanf failed");
continue;
@@ -512,6 +517,8 @@ bool MavlinkLogHandler::log_entry_from_id(uint16_t log_id, LogEntry *entry)
continue;
}
// Parse log entry with width specifier to prevent buffer overflow
// %255s = max 255 chars + null = 256 bytes (LOG_FILEPATH_SIZE)
if (sscanf(line, "%" PRIu32 " %" PRIu32 " %255s", &(entry->time_utc), &(entry->size_bytes), entry->filepath) != 3) {
PX4_DEBUG("sscanf failed");
continue;
+3 -1
View File
@@ -48,12 +48,14 @@ public:
void handle_message(const mavlink_message_t *msg);
private:
static constexpr size_t LOG_FILEPATH_SIZE = 256; // Buffer size for log file paths
struct LogEntry {
uint16_t id{0xffff};
uint32_t time_utc{};
uint32_t size_bytes{};
FILE *fp{nullptr};
char filepath[256];
char filepath[LOG_FILEPATH_SIZE];
uint32_t offset{};
};