convert_old_parameter(), convert_class() and _convert_parameter_width()
each declared a stack buffer sized by type_size() of a runtime parameter
type. type_size() can return zero (AP_PARAM_NONE/GROUP, or an unknown
type), which makes the array a zero-length VLA - undefined behaviour -
and VLAs are non-standard C++ in any case.
Replace the VLAs with an object of a union of every storable parameter
type. Such an object is large enough to hold any single parameter value
without assuming which type is largest, and is correctly aligned for the
typed accesses made through the AP_Param pointer.
The buffer's size had also been used as a length: in
_convert_parameter_width() as the EEPROM read length, and in
convert_old_parameter() and convert_class() as the same-type copy
length. The union is not exactly the size of the value it holds, so
size those operations with type_size() explicitly, preserving the
lengths the VLAs gave. Using sizeof() on the union instead would read
past the stored parameter, and in the two copy cases write up to
sizeof(Vector3f) bytes into a destination which may be as small as one
byte.
This clears the clang-scan-build core.VLASize findings.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
buflen now includes the space required for the null terminator, so up
to buflen-1 characters are returned. Previously fgets could write
buf[buflen], one byte beyond the caller-nominated length. All
in-tree callers passed sizeof(buf)-1 to compensate, but
posix_compat's apfs_fgets forwards its C-style size argument
directly, so a caller supplying a size-byte buffer could have that
buffer overrun by one byte on an over-long line.
Update callers to pass the full buffer size; usable capacity is
unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove any default_list entries that are superseded by param_overrides.
Constructors (e.g. SRV_Channels) can call add_default() before
param_overrides is populated, leaving stale nodes that would cause
check_default() to shadow the correct override value.
top level parameters don't have flags, but we need to fill in the
flags return field as otherwise we can refuse to set a parameter based
on it being internal and the allow_set_via_mavlink() check
if we can't save a parameter due to the queue size not being large
enough then there is a coding error, likely the code trying to save
large numbers of parameters while armed
the cygwin build is not generating binaries failing with:
undefined reference to `AP_Param::load_param_defaults(char const volatile*, int, bool)
there is a 2nd problem that the CI test for cygwin doesn't fail when
the build fails. That will be addressed separately
when we load a VARPTR subtree we need to re-scan the parameter
defaults file from @ROMFS/defaults.parm in case there are defaults
applicable to this subtree
this fixes an issue with resetting of parameters when going between
4.4.x and 4.5.x on MatekH743, and on any other board using flash
storage where the storage size has increased from 16k to 32k between
4.4.x and 4.5.x
The problem is that when you update to 4.5.x the parameter code stored
a backup of parameters in the StorageParamBak storage region which is
in the last section of storage. When you downgrade to 4.4.x the
AP_FlashStorage::load_sector() code tries to load this data and gets
an error as it is beyond the end of the available 16k storage. This
triggers an erase_all() and loss of parameters
* This was undefined behavior in the C++ standard
* Use the safer options in AP_Common
* Removes a compiler warning
Signed-off-by: Ryan Friedman <ryanfriedman5410+github@gmail.com>
Peter BarkerandClaude Opus 4.8
LachlanConn
Thomas Watson
Andrew Tridgell
Iampete1
muramura
Andy Piper
Tim Tuxworth
Ryan Friedman