Update linked-lists functions#1422
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1422 +/- ##
==========================================
+ Coverage 91.34% 91.64% +0.30%
==========================================
Files 36 36
Lines 3997 4010 +13
Branches 507 508 +1
==========================================
+ Hits 3651 3675 +24
+ Misses 256 245 -11
Partials 90 90
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request removes boolean return values from linked-list functions in favor of void returns, representing a breaking API change. The functions now silently handle error conditions (e.g., NULL parameters, operations on empty lists) rather than returning success/failure indicators.
Changes:
- Changed 16 list manipulation functions from returning
boolto returningvoid(push, pop, insert, remove, clear, sort, reverse operations) - Updated function implementations to handle errors silently via early returns instead of returning false
- Optimized
flist_empty()andlist_empty()from O(n) to O(1) by directly checking NULL instead of calling size functions - Updated all tests to remove boolean return value checks
- Updated extensive documentation in header files and mainpage.dox to reflect new API
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| lib_lists/lists.h | Changed function signatures from bool to void return types, removed @return documentation for affected functions |
| lib_lists/lists.c | Updated all internal and public functions to use void returns with silent error handling, optimized empty() functions to O(1) |
| unit-tests/lib_lists/test_lists.c | Removed assert_true/assert_false checks on function return values, removed tests for calling pop on empty lists |
| lib_lists/doc/mainpage.dox | Extensive updates to examples and best practices reflecting new void-return API and updated error handling guidance |
| fuzzing/harness/fuzzer_lists.c | Updated fuzzing harness to remove boolean return value checks |
Comments suppressed due to low confidence (1)
lib_lists/doc/mainpage.dox:80
- This statement contradicts the performance table on line 72 which correctly states that insert/remove at back is O(n) for doubly-linked lists. The implementation confirms this is O(n) since push_back_internal traverses the entire list to find the tail. This misleading statement should be removed or corrected to accurately reflect that doubly-linked lists do NOT provide O(1) tail operations in this implementation.
**Best practice:** Use forward lists when memory is critical and you don't need backward
traversal or frequent tail operations. Use doubly-linked lists when you need bidirectional
access or O(1) tail operations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6d96aa1 to
00add2d
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
lib_lists/doc/mainpage.dox:80
- Misleading documentation about tail operations. The text says "Use doubly-linked lists when you need bidirectional access or O(1) tail operations" but according to the table above (line 72) and the implementation, both list types have O(n) tail operations (push_back and pop_back), not O(1). This reference to O(1) tail operations should be removed or corrected to say "bidirectional access" only.
**Best practice:** Use forward lists when memory is critical and you don't need backward
traversal or frequent tail operations. Use doubly-linked lists when you need bidirectional
access or O(1) tail operations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
00add2d to
04f30c6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 10 comments.
Comments suppressed due to low confidence (2)
unit-tests/lib_lists/test_lists.c:501
- The comment says
list pop_back (O(1) - fast!), butlist_pop_backcallsflist_pop_back, which traverses to find the penultimate node (O(n)). Update the test comment so it doesn’t contradict the implementation and the library docs.
// Test: list pop_back (O(1) - fast!)
static void test_list_pop_back(void **state)
{
lib_lists/doc/mainpage.dox:81
- The guidance here mentions needing doubly-linked lists for "O(1) tail operations", but this library’s
list_push_back/list_pop_backare O(n) (they traverse the list). Please update this sentence to avoid contradicting the complexity table and the actual implementation.
**Best practice:** Use forward lists when memory is critical and you don't need backward
traversal or frequent tail operations. Use doubly-linked lists when you need bidirectional
access or O(1) tail operations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
04f30c6 to
0d9f527
Compare
0d9f527 to
5325aaa
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5325aaa to
fdaa687
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
lib_lists/doc/mainpage.dox:80
- The documentation states that doubly-linked lists provide "O(1) tail operations", but the table above correctly shows that insert/remove at back operations are O(n) for doubly-linked lists. This is inconsistent and misleading. The text should be updated to remove the mention of "O(1) tail operations" or clarify that tail operations are also O(n) for the doubly-linked list implementation.
**Best practice:** Use forward lists when memory is critical and you don't need backward
traversal or frequent tail operations. Use doubly-linked lists when you need bidirectional
access or O(1) tail operations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fdaa687 to
6557a53
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
lib_lists/doc/mainpage.dox:80
- The best practice text states "Use doubly-linked lists when you need bidirectional access or O(1) tail operations" but the implementation and the updated documentation both show that tail operations (push_back, pop_back) are O(n) for doubly-linked lists, not O(1). This statement should be updated to remove the mention of O(1) tail operations or clarify what is meant.
**Best practice:** Use forward lists when memory is critical and you don't need backward
traversal or frequent tail operations. Use doubly-linked lists when you need bidirectional
access or O(1) tail operations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6557a53 to
fbcf475
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Changes include
Auto cherry-pick in API_LEVEL
If requested to port the commits from this PR on a dedicated API_LEVEL branch,
select the targeted one(s), or add new references if not listed:
[x] TARGET_API_LEVEL: API_LEVEL_25
This will only create the PR with cherry-picks, ready to be reviewed and merged.
Remember: