Fix compiler warnings in timing and function removal code#1191
Fix compiler warnings in timing and function removal code#1191gaoxueyu merged 9 commits intoIvorySQL:masterfrom
Conversation
📝 WalkthroughWalkthroughConsolidated local declarations and adjusted control flow/assertions in several PL/PLiSQL functions; initialized a timing accumulator base in psql; switched a string copy to memcpy with explicit null termination. No exported/public API signatures changed. (≈40 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@contrib/ivorysql_ora/src/builtin_packages/dbms_utility/dbms_utility.c`:
- Line 69: The variable `fn` is declared outside the WIN32 guard and will be
unused on Windows; move the declaration of `fn` back inside the `#ifndef WIN32`
block (or, alternatively, add a suppression like `(void) fn;` under `#ifdef
WIN32` or use a compiler unused attribute) so Windows builds do not get an
unused-variable warning—update the declaration near where `fn` is first used in
the guarded code (referencing the `fn` identifier in this file).
In `@src/bin/psql/common.c`:
- Line 2863: The code sets the instr_time variable `before` by directly
assigning `before.ticks = 0;`, which couples to the internal representation;
replace that direct member access with the provided macro call
`INSTR_TIME_SET_ZERO(before)` to match other uses (e.g., DescribeQuery and
ExecQueryAndProcessResults) and avoid fragile direct `.ticks` manipulation.
🧹 Nitpick comments (1)
src/pl/plisql/src/pl_subproc_function.c (1)
3045-3050:Assert(false)is a no-op in production builds — considerelog(ERROR, …)for defense-in-depth.If the loop in
plsql_init_glovalvar_from_stack(and the identical pattern inplsql_assign_out_glovalvar_from_stackat line 3079) ever fails to find a matching parent frame in a non-assert build, execution silently falls through and the global variable is left uninitialized. An explicitelog(ERROR, "could not find parent frame for datum %d", dno)would surface the problem instead of hiding it.Suggested change
- Assert(false); - return; + elog(ERROR, "could not find parent frame for datum %d", dno);Apply the same pattern at line 3079.
|
Great quality improvement！ |
…h of the source argument
This PR fixes all the warnings in master branch introduced in recent months.
Summary by CodeRabbit
No changes to public interfaces or user-facing behavior.