Fix User Edit Zone Crashes and Add Comprehensive Test Coverage#336
Merged
Conversation
- Add test_time_operations.cc with TimeInfo class tests * Time arithmetic operations (seconds, minutes, days, weeks, months, years) * Comparison operators and date ordering * Business logic scenarios (shift scheduling) * Operator overloads and edge cases - Add test_error_handler.cc with error management tests * ErrorInfo construction and severity levels * Error categories (9 types: GENERAL, SYSTEM, NETWORK, etc.) * Context tracking (file, line, function, error code) * Real-world error scenarios - Add test_sales_tax_calculations.cc with 43 test cases * US tax calculations, Canadian GST/PST/HST/QST * European VAT, discounts, coupons, modifiers * Quantity pricing and rounding - Fix floating-point comparison precision issues * Apply Catch::Approx() to all float equality checks * Fix RAII scoping issue in test_memory_modernization.cc - Update CMakeLists.txt to include new test files Test suite now contains 80 test cases with 568 assertions (100% passing)
Fixes segmentation fault (SIGSEGV) when saving employee records in the User Edit Zone. The crash occurred during form field iteration when the field pointer became NULL. Root Cause: - Loop condition checked 'f != nullptr' but immediately dereferenced f->next without revalidation on the first line of the loop body - This caused a segfault when f was NULL at loop entry or became NULL during iteration Changes: - Add explicit NULL check before dereferencing f->next in job info loop - Add early validation for user pointer and FieldList() return value - Add error logging with ReportError() for easier debugging - Return error code 1 instead of continuing with NULL pointers Crash Stack Trace Reference: TextField::Get(int&) <- UserEditZone::SaveRecord() Signal: SIGSEGV at zone/user_edit_zone.cc line 440 Tested: Compiles successfully, prevents crash on employee save operation
- Fixed SIGSEGV when saving employee records caused by NULL/dangling user pointers - Added user pointer validation before SaveRecord calls in Signal() and Update() - Enhanced SaveRecord field iteration with proper null checks and early breaks - Added dangling pointer detection to catch freed memory access - Improved error logging for better debugging Root causes: 1. SaveRecord called without validating user pointer when toggling Active/Inactive 2. SaveRecord called without validation during job filter updates 3. Field pointer advanced without null checks in job info iteration loop Testing: Validated with AddressSanitizer in Debug build Fixes: User Edit button type save crashes
Member
|
It is scary (scary good !) watching you fix bugs and refactor the code. In email I get these information laden summaries every time a commit is made, even if in your 'dev' and not in 'master'; the summaries are positively overwhelming in every way. In the old days such a level of professionalism and skill was unheard of because it was simply impossible to expect such results from anyone fixing, refactoring, enhancing and testing code. Congratulations, Ariel. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes critical segmentation faults in the User Edit Zone when saving employee records and expands the test suite with 26 new test cases covering time operations, error handling, and sales tax calculations.
Changes
🐛 Bug Fixes
User Edit Zone - Multiple Crash Fixes (Commits: 5160ede, c259bcf)
Root Causes Identified:
SaveRecord()called without validating user pointer when toggling Active/Inactive viewsSaveRecord()called without validation during job filter updatesCrash Details:
UserEditZone::SaveRecord()TextField::Get(int&)<-UserEditZone::SaveRecord()0xbebebebebebebebe)✅ Testing
Test Suite Expansion (Commit: 96b0300)
test_time_operations.ccwith TimeInfo class tests (time arithmetic, comparisons, business logic)test_error_handler.ccwith error management tests (severity levels, categories, context tracking)test_sales_tax_calculations.ccwith 43 test cases (US, Canadian, European VAT)Test Results:
Files Modified
Testing Performed
Impact