Skip to content

docs: verify no padding issues in serialized structs#66

Closed
gdellis wants to merge 1 commit into
mainfrom
fix/struct-padding-audit
Closed

docs: verify no padding issues in serialized structs#66
gdellis wants to merge 1 commit into
mainfrom
fix/struct-padding-audit

Conversation

@gdellis
Copy link
Copy Markdown
Owner

@gdellis gdellis commented Apr 19, 2026

Summary

Audit of structs serialized via memcpy in the codebase for issue #63.

Audit Results

BLE structs (already fixed in PR #52):

  • BleLocationData (17 bytes) - uses #pragma pack(push, 1), static_assert added
  • BleAlertData (18 bytes) - uses #pragma pack(push, 1), static_assert added

GPS structs (no issue):

  • GpsData - Contains only C++ fundamental types (double, uint8_t, uint64_t), no memcpy serialization found. Used internally, not serialized over wire.

LoRa structs (no issue):

  • LoRaPacket - Contains fixed-size array (uint8_t data[255]), no struct memcpy. Uses manual byte copying in send/receive methods.

Manual serialization (verified correct):

  • try_lora_send() in state_machine.cpp manually copies fields with known sizes (4-byte int32, 2-byte battery, etc.) - no struct padding concern
  • try_ble_fallback() populates BleLocationData field-by-field, not via memcpy

Conclusion

No additional structs need #pragma pack. The BLE structs were the only ones serialized via struct memcpy, and those are already fixed.

Closes #63

- Add static_assert to verify BLE struct sizes (17 bytes for BleLocationData, 18 for BleAlertData)
- Add [[nodiscard]] to all esp_err_t returning methods to catch ignored errors at compile time
- Fix state_machine.cpp to use ESP_ERROR_CHECK on ble_.send_alert() call

Closes #61, #62
@gdellis
Copy link
Copy Markdown
Owner Author

gdellis commented Apr 19, 2026

This PR audited other serialized structs for padding issues (issue #63). The audit found no additional issues beyond those already fixed in PR #52.

The branch has been rebased onto current main and no additional changes are needed. Closing as duplicate of the fixes already merged.

@gdellis gdellis closed this Apr 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

enhancement: Check other serialized structs for padding issues

1 participant