Skip to content

Coverity fixes 25.10 #6075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Coverity fixes 25.10 #6075

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd353a8
Coverity fixes 25.10
szalpal Oct 24, 2025
2973d0c
Apply suggestions from code review
szalpal Oct 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions dali/kernels/signal/window/extract_windows_gpu.cuh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ struct ExtractHorizontalWindowsImplGPU : ExtractWindowsImplGPU<Dst, Src> {
if (out_win_length > args.window_length && !concatenate) {
int max_pad_block_x = 32;
if (max_win_per_input < 32) {
assert(max_win_per_input != 0);
Copy link
Contributor

@mzient mzient Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion is already in place (line 531) assert(max_win_per_input >= 1); and max_win_per_input is not modified in between. Mark it as a false positive and don't clutter the code with redundant checks.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't agree. This coverity issue will reappear, therefore for the sake of making next iterations smoother it's better to have the assertion right there. Additionally, it doesn't harm release build. Therefore I'm keeping it here.

Copy link
Contributor

@mzient mzient Oct 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This coverity issue will reappear

Why? Coverity remembers those. We have tons of false positives marked and they don't bother us. Additionally, I'm afraid that this can actually trigger an "unreachable code" warning in the compiler in debug buidls.
Also, this assertion doesn't make this code in any way more secure or anything - intiger division by zero is a hard fault and causes the program to abort, just like assert does, so we gain nothing even in debug builds.

max_pad_block_x = 1024/max_win_per_input;
}
int pad_block_x = clamp(pad_length, 1, max_pad_block_x);
Expand Down
4 changes: 2 additions & 2 deletions dali/python/backend_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ void FillTensorFromDlPack(

size_t bytes = volume(shape) * dali_type.size();

auto typed_shape = ConvertShape(shape, batch);
const auto & typed_shape = ConvertShape(shape, batch);
bool is_pinned = dl_tensor.device.device_type == kDLCUDAHost;
int device_id = CPU_ONLY_DEVICE_ID;
// according to the docs kDLCUDAHost = kDLCPU | kDLCUDA so test it as a the first option
Expand Down Expand Up @@ -328,7 +328,7 @@ void FillTensorFromCudaArray(const py::object &object,
CheckContiguousTensor(strides, shape, type.size());
}

auto typed_shape = ConvertShape(shape, batch);
const auto & typed_shape = ConvertShape(shape, batch);
auto *ptr = PyLong_AsVoidPtr(cu_a_interface["data"].cast<py::tuple>()[0].ptr());

// it is for __cuda_array_interface__ so device_id < 0 is not a valid value
Expand Down
Loading