Reduce Optimization Warnings - #132
Conversation
Clean up.
…lls to strtcpy(). Include cxlib/strtcpy.h where needed. Update copyright notices. Clean up.
|
I haven't fixed all the warnings yet. I'll mark this as ready for review once more of them are resolved. |
|
I finished resolving all the warnings when passing |
|
@greptileai Please review. |
Greptile SummaryThis PR reduces optimization-build warnings and hardens several previously undefined or unchecked paths.
Confidence Score: 5/5The PR appears safe to merge, with no concrete changed-code regression identified. The behavior-affecting changes add error propagation, initialization, bounds handling, and cleanup safeguards, while the remaining edits are warning-oriented type corrections or removal of unused state.
|
| Filename | Overview |
|---|---|
| centrallix/cxss/cxss_credentials_db.c | Initializes credential-list state, checks allocation and SQLite binding failures, and safely cleans up partial results. |
| centrallix/expression/exp_functions.c | Rejects unsupported hash algorithms and initializes aggregate evaluation status before use. |
| centrallix/objectsystem/obj_datatypes.c | Bounds money fractional formatting and reports malformed fractional values without changing valid-value behavior. |
| centrallix/wgtr/apos.c | Propagates recursive layout failures and removes unused bookkeeping while preserving layout side effects. |
| centrallix-lib/src/mtask.c | Locally suppresses intentional uninitialized-stack warnings required by the green-thread stack setup. |
| centrallix/utility/json_util.c | Removes unused datetime-presence tracking while retaining parsing and accepted-field behavior. |
Reviews (1): Last reviewed commit: "Add comments to make dead code explicit." | Re-trigger Greptile
|
This PR is cleared for human review. |
|
Too many files changed for review (136 files, 100 file limit). Bypass the limit by tagging |
|
🎉 All dependencies have been resolved ! |
|
Is there a way to refresh this PR so it doesn't show those 114 or so changed files from PR #95 which has already been merged? |
|
Yup, turns out merging |
|
I conducted my own review and fixed several bugs. This PR looks fully ready to merge. |
gbeeley
left a comment
There was a problem hiding this comment.
Fix needed for modulus clamping of fraction_part.
| } | ||
|
|
||
| /** Clamp the cents to the low four digits **/ | ||
| fraction_part = m->FractionPart % 10000; |
There was a problem hiding this comment.
This will be better resolved once we merge the MoneyType representation PR, but I think this change makes the problem worse, though the warning message helps. It prevents weird output from a malformed MoneyType value, but it does so by hiding the malformed data and making it look like everything's okay. It'd be better to either have it print a malformed value weirdly, fold the out of range fraction_part into the integer_part, or else to return an error. The latter may be best here.
There was a problem hiding this comment.
Alright, switching to returning an error.
Reduce more warnings that only occur when
--enable-optimizationis used. Some of these warning fixes also fix minor bugs and improve hardening.GitHub Relationships
These will need to be reviewed and merged into master before this PR is reviewed.