Fix for static analysis issues #134
Conversation
Sapna1-singh
force-pushed
the
ww30_minigbm_covfix
branch
2 times, most recently
from
4cd2980
to
3288183
Compare
| @@ -403,7 +403,9 @@ static int cross_domain_bo_create(struct bo *bo, uint32_t width, uint32_t height | |||
| drv_loge("DRM_VIRTGPU_RESOURCE_CREATE_BLOB failed with %s\n", strerror(errno)); | |||
| return -errno; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Add space before and after if block
| if (vma->cpu) { | ||
| free(vma->addr); | ||
| vma->addr = NULL; | ||
| } | ||
| return munmap(vma->addr, vma->length); | ||
| if (vma->addr != NULL) | ||
| ret = munmap(vma->addr, vma->length); |
There was a problem hiding this comment.
Check spaces/tab used, ideally it should be 4 spaces
There was a problem hiding this comment.
I have followed the indentation used before as reference. In this function, it was tab space that was used before, so i have followed the same.
cros_gralloc/cros_gralloc_driver.cc
Outdated
| @@ -179,7 +184,7 @@ cros_gralloc_driver::cros_gralloc_driver() | |||
| switch (availabe_node) { | |||
| // only have one render node, is GVT-d/BM/VirtIO | |||
| case 1: | |||
| if (!drv_render_) { | |||
| if (drv_render_) { | |||
There was a problem hiding this comment.
Seems we are altering the code functionality here, are we sure about this code change?
| @@ -968,7 +968,7 @@ Return<void> CrosGralloc4Mapper::dumpBuffer(const cros_gralloc_buffer* crosBuffe | |||
| auto metadata_get_callback = [&](Error, hidl_vec<uint8_t> metadata) { | |||
| MetadataDump metadataDump; | |||
| metadataDump.metadataType = metadataType; | |||
| metadataDump.metadata = metadata; | |||
| metadataDump.metadata = std::move(metadata); | |||
There was a problem hiding this comment.
remove extra space before std::move(metadata);
drv_helpers.c
Outdated
| @@ -522,11 +522,14 @@ void *drv_dumb_bo_map(struct bo *bo, struct vma *vma, uint32_t map_flags) | |||
|
|
|||
| int drv_bo_munmap(struct bo *bo, struct vma *vma) | |||
| { | |||
| int ret = 0; | |||
There was a problem hiding this comment.
0 seems to be a success case for munmap
Should the default value be -1 ?
| @@ -492,8 +492,8 @@ static int i915_add_combinations(struct driver *drv) | |||
| static int i915_align_dimensions(struct bo *bo, uint32_t format, uint32_t tiling, uint32_t *stride, | |||
| uint32_t *aligned_height) | |||
| { | |||
| uint32_t horizontal_alignment = 64; | |||
| uint32_t vertical_alignment = 4; | |||
| uint32_t horizontal_alignment; | |||
There was a problem hiding this comment.
Are you sure you want to leave these values uninitialized?
There was a problem hiding this comment.
yes. the value of horizontal_alignment gets updated on every condition mentioned here. And whatever we initialize here, will never be used. That was the reason it caused coverity error "Unused Value".
Sapna1-singh
force-pushed
the
ww30_minigbm_covfix
branch
from
3288183
to
198f942
Compare
Below are the issues fixed: - Uninitialized scalar variable - Use after free - Resource leak - Explicit null dereferenced - Unused value - Rule of three - COPY_INSTEAD_OF_MOVE - Dereference after null check - Dereference null return value - Untrusted loop bound - Use after free - Missing assignment operator Tracked-On: OAM-122342 Signed-off-by: Sapna <[email protected]>
Sapna1-singh
force-pushed
the
ww30_minigbm_covfix
branch
from
198f942
to
d468864
Compare
sysopenci
requested review from
feijiang1,
gkdeepa,
renchenglei,
YuanjunHuang and
yyao3
|
These coverity issues are related upstream code, then we can directly get waiver for them, don't need fix actually. If we need fix them, it is better directly submit to upstream repo. |
Below are the issues fixed:
Tracked-On: OAM-122342