Fix retrieve user value in IOCTL_VALSET_NUM case#359
Conversation
jserv
left a comment
There was a problem hiding this comment.
Remove "fix: " in the subject of git commit messages.
Refine git commit message, making it more informative. See https://chris.beams.io/git-commit
Don't close this pull request. Use git force-push instead.
examples/ioctl.c
Outdated
|
|
||
| case IOCTL_VALSET_NUM: | ||
| ioctl_num = arg; | ||
| int user_val; |
There was a problem hiding this comment.
To be compatible with the old version, which might still use the C89 standard.
Don't declare the variable inside the switch statement.
Or, you should declare the variable within the block scope.
Also, if you want to declare the variable inside the switch statement, please fix this:
ioctl.c:78:9: error: typename in expression
ioctl.c:78:13: error: Expected ; at end of statement
ioctl.c:78:13: error: got user_val
ioctl.c:78:9: error: undefined identifier 'int'
ioctl.c:79:13: error: undefined identifier 'user_val'
ioctl.c:81:21: error: undefined identifier 'user_val'
|
Hello, I force-pushed a new commit. Please review it. |
Github notifies the relevant people automatically, so there's no need to leave a comment like this. |
|
Thanks for the patch. While this addresses the pointer-to-value mismatch, should we consider whether Currently, the macro is defined as: Using |
jserv
left a comment
There was a problem hiding this comment.
Ensure consistent code style with 'clang-format' in examples directory.
|
Yes, using _IO with pass-by-value is more idiomatic and avoids unnecessary memory copying. |
|
Wouldn't the proposed patch also change the existing ABI? |
|
Hello, I'm uncertain whether to add more code as shown below or simply rewrite the original. #define IOCTL_VALSET_NUM2 _IO(IOC_MAGIC, 4)
#define IOCTL_VAL_MAXNR 4
...
case IOCTL_VALSET_NUM2:
ioctl_num = (int)arg;
break; |
|
Based on the int ioctl_get_nth_byte(int file_desc)
{
int i, c;
printf("get_nth_byte message:");
i = 0;
do {
c = ioctl(file_desc, IOCTL_GET_NTH_BYTE, i++);
/* ... */The test program in the LKMPG only passes the value to the ioctl. Please either change the test program or leave the ioctl logic unchanged. |
766280b to
68fc022
Compare
Replace _IOW with _IO macro to reflect that IOCTL_VALSET_NUM passes the integer value directly via arg, consistent with the calling convention used throughout this codebase.
|
Thank @applepwc for contributing! |
The test program is:
Summary by cubic
Switch IOCTL_VALSET_NUM to _IO so the integer is passed via the ioctl arg, matching the driver handler and example calls. This fixes the set/get number flow and keeps the convention consistent across the codebase.
Written for commit 113cd57. Summary will update on new commits.