Fix Coverity issue in inliner plugin.#10442
Conversation
|
I think this looks good, but I'd like @cmcfarlen to review. |
| ats::io::IO *const io = new io::IO(); | ||
| std::unique_ptr<ats::io::IO> io{new io::IO()}; | ||
|
|
||
| TSDebug(PLUGIN_TAG, "request:\n%s", request.c_str()); |
There was a problem hiding this comment.
It's not clear what exactly Coverity's gripe is. My best guess is that it's seeing that *io would get leaked if TSDebug() threw an exception. But there must be a bazillion cases of that in our code, why would it single this one out? Are there instructions anywhere about how to run Coverity, to be sure we've appeased it?
There was a problem hiding this comment.
No unfortunately we don’t have enough karma to run PRs against coverity. So it’ll have to land on master for the next schedule run, rinse repeats…
There was a problem hiding this comment.
It looks like from reading the defect report, coverity doesn't trace into the second get call to see that it is indeed assigned eventually. The std::move will hopefully help it realize that ownership is handed off.
cmcfarlen
left a comment
cmcfarlen
left a comment
There was a problem hiding this comment.
This looks good to me.
| request += "\r\n\r\n"; | ||
|
|
||
| ats::io::IO *const io = new io::IO(); | ||
| std::unique_ptr<ats::io::IO> io{new io::IO()}; |
There was a problem hiding this comment.
No strong opinions on this nitpick, but this could be auto io = std::make_unique<io::IO>();
| TSDebug(PLUGIN_TAG, "request:\n%s", request.c_str()); | ||
|
|
||
| ats::get(io, io->copy(request), AnotherClass(src_)); | ||
| auto r{io->copy(request)}; |
There was a problem hiding this comment.
I had to check what copy returned. A better name for r might be size or sz.
| ats::io::IO *const io = new io::IO(); | ||
| std::unique_ptr<ats::io::IO> io{new io::IO()}; | ||
|
|
||
| TSDebug(PLUGIN_TAG, "request:\n%s", request.c_str()); |
There was a problem hiding this comment.
It looks like from reading the defect report, coverity doesn't trace into the second get call to see that it is indeed assigned eventually. The std::move will hopefully help it realize that ownership is handed off.
|
[approve ci autest] |
* asf/master: (30 commits) add conveinience function to lookup name->IntType* (apache#10474) cmake: bigobj subdir has executables, not plugins (apache#10481) cmake: compile jsonrpc_protocol with -fPIC (apache#10478) Make sure new metrics are always considered (apache#10445) Refactor and rename restart metrics (apache#10472) Fix the SNI and HOST parsing properly (apache#10480) slice/Data.h: CID 1508924: Uninitialized scalar field (apache#10470) CID 1508882: initialize pointer (apache#10469) CID-1512726: Mute coverity, use explicit check for iterator use past end (apache#10467) Fix Coverity issue in inliner plugin. (apache#10442) Set the proper variable when find_package fails to find the package (apache#10468) CID1508860: double lock confusion (apache#10443) Stop using functions that are unavailable on the latest quiche (apache#10447) Add allow-plain server ports attribute (apache#9574) [Fuzzing] move build.sh in trafficserver (apache#10466) Some sort of "fix" to mute coverity. (apache#10451) CID-1512733: Fix coverity issue (apache#10452) Fix cmake autooptions (apache#10456) Cmake presets (apache#10457) This fixes CID 1518257 (apache#10404) ...
3 participants
CID 1508893