-
Notifications
You must be signed in to change notification settings - Fork 618
drivers/compression: fix decompress ioctl #6894
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.
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
drivers/compression: fix decompress ioctl #6894
Conversation
Add a break statement to the COMPIOC_DECOMPRESS case in comp_ioctl. This fix ensures proper handling of decompression requests. Signed-off-by: seokhun-eom <[email protected]>
pcs1265
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
jeongarmy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
| return -EINVAL; | ||
| } | ||
| ret = decompress_block(comp_info->output_buffer, &comp_info->output_size, comp_info->input_buffer, &comp_info->input_size); | ||
| if (ret == ENOMEM) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error return valuse must be negative.
Could you please check return values of decompress_block and change to negative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the decompress_block function the return of ENOMEM indicates that the output buffer was insufficient, resulting in only a portion of the data being decompressed while the remainder remained compressed.
In the LZMA algorithm this scenario is designed to return OK, whereas the MINIZ algorithm returns a Z_BUF_ERROR.
Would it be appropriate to modify this to return an error instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed it from new PR.
#6911
| if (ret == ENOMEM) { | ||
| bcmpdbg("Output buffer allocated is not sufficient\n"); | ||
| } | ||
| break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not related to this pr but please check code line where return just ERROR, it is wrong. each of ioctl should return one of errno.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for review. I'll make another PR for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a new PR from your comment.
#6911
| if (ret == ENOMEM) { | ||
| bcmpdbg("Output buffer allocated is not sufficient\n"); | ||
| } | ||
| break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not related to this pr but please check code line where return just ERROR, it is wrong. each of ioctl should return one of errno.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR fix the required decompression reqin comp_ioctl
Add a break statement to the COMPIOC_DECOMPRESS case in comp_ioctl.
This fix ensures proper handling of decompression requests.