posix-clock: Use an unsigned data type for a variable
Subsystems (2)
PATCH 1/1 posix-clock: Use an unsigned data type for a variable
From: Markus Elfring <[email protected]>
Date: Sun, 20 Dec 2015 09:09:34 +0100
The data type "int" was used by the variable "result" in the function "posix_clock_poll" even though the type "uint" will usually be needed for the return value from a call of the function which was assigned to the pointer "poll" of the variable "clk". Reuse the type from this poll call instead.
This issue was detected by using the Coccinelle software.
--- kernel/time/posix-clock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/time/posix-clock.c +1 -1
@@ -69,7 +69,7 @@ static ssize_t posix_clock_read(struct file *fp, char __user *buf,static unsigned int posix_clock_poll (struct file *fp,poll_table *wait) {struct posix_clock *clk =get_posix_clock (fp); -int result =0 ; +uint result =0 ;if (!clk)return -ENODEV ;
Signature · 1 line
2.6.3
Quoted 8 lines · SF Markus Elfring
From: Markus Elfring <[email protected]>
Date: Sun, 20 Dec 2015 09:09:34 +0100The data type "int" was used by the variable "result" in the function "
posix_clock_poll" even though the type "uint" will usually be needed for the return value from a call of the function which was assigned to the pointer "poll" of the variable "clk". Reuse the type from this poll call instead.
Why use uint when the function return type it unsigned int?
On the other hand, why is the function return type unsigned int when there is a return of a negative constant?
julia
Quoted 29 lines
This issue was detected by using the Coccinelle software.
Signed-off-by Markus Elfring <[email protected]>--- kernel/time/posix-clock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)kernel/time/posix-clock.c +1
-1@@ -69,7 +69,7 @@ static ssize_t posix_clock_read(struct file *fp, char __user *buf,static unsigned int posix_clock_poll (struct file *fp,poll_table *wait) {struct posix_clock *clk =get_posix_clock (fp); -int result =0 ; +uint result =0 ;if (!clk)return -ENODEV ;Signature · 6 lines
2.6.3 -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 3 lines
Quoted 1 line
Reuse the type from this poll call instead.
Why use uint when the function return type it unsigned int?
Do you prefer to express the type modifier once more there?
Quoted 2 lines
On the other hand, why is the function return type unsigned int when there is a return of a negative constant?
This implementation detail can trigger further software development considerations, can't it?
This is a general issue for the exception handling in several functions. Would you like to improve any more source code around it?
Regards,
Markus
Quoted 5 lines · SF Markus Elfring
Quoted 3 lines
Quoted 1 line
Reuse the type from this poll call instead.
Why use uint when the function return type it unsigned int?
Do you prefer to express the type modifier once more there?
I don't know what the sentence means, but I think that the type should be referenced in a consistent manner.
Quoted 5 lines
Quoted 2 lines
On the other hand, why is the function return type unsigned int when there is a return of a negative constant?
This implementation detail can trigger further software development considerations, can't it?
It would seem reasonable to address all of the signed/unsigned issues related to the function return value at once.
julia
Quoted 10 lines
This is a general issue for the exception handling in several functions. Would you like to improve any more source code around it?
Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
Reuse the type from this poll call instead.
Why use uint when the function return type it unsigned int?
Do you prefer to express the type modifier once more there?
I don't know what the sentence means,
Can it be a matter of taste if the key word "unsigned" should be repeated in such an use case?
Quoted 1 line
but I think that the type should be referenced in a consistent manner.
How do involved software designers and developers prefer to achieve data type consistency here?
Which kind of naming convention will get priority?
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
On the other hand, why is the function return type unsigned int when there is a return of a negative constant?
This implementation detail can trigger further software development considerations, can't it?
It would seem reasonable to address all of the signed/unsigned issues related to the function return value at once.
Would you like to extend another evolving script for the semantic patch language?
I imagine that the general issue around the exception handling will cause too many software development challenges to tackle them "at once".
Regards,
Markus
Quoted 10 lines · SF Markus Elfring
From: Markus Elfring <[email protected]>
Date: Sun, 20 Dec 2015 09:09:34 +0100The data type "int" was used by the variable "result" in the function "
posix_clock_poll" even though the type "uint" will usually be needed for the return value from a call of the function which was assigned to the pointer "poll" of the variable "clk". Reuse the type from this poll call instead.This issue was detected by using the Coccinelle software.
Coccinelle has pointed out a type mismatch. This fact may or may not indicate a real problem. You need to analyze code to see if there is a problem and then fix the problem.
In this case, there is indeed a problem. However, just changing the type won't fix it.
Thanks,
Richard
Quoted 2 lines
In this case, there is indeed a problem. However, just changing the type won't fix it.
Would you like to explain your view on the handling of potential data type mismatches a bit more?
Regards,
Markus
Quoted 5 lines · SF Markus Elfring
Quoted 2 lines
In this case, there is indeed a problem. However, just changing the type won't fix it.
Would you like to explain your view on the handling of potential data type mismatches a bit more?
Yes, I will explain in the patch that follows.
Thanks,
Richard
PATCH 1/1 posix-clock: fix return code on the poll method's error path
In reply to SF Markus ElfringThe posix_clock_poll function is supposed to return a bit mask of POLLxxx values. However, in case the hardware has disappeared (due to hot plugging for example) this code returns -ENODEV in a futile attempt to throw an error at the file descriptor level. The kernel's file_operations interface does not accept such error codes from the poll method. Instead, this function aught to return POLLERR.
The value -ENODEV does, in fact, contain the POLLERR bit (and almost all the other POLLxxx bits as well), but only by chance. This patch fixes code to return a proper bit mask.
Credit goes to Markus Elfring for pointing out the suspicious signed/unsigned mismatch.
--- kernel/time/posix-clock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/time/posix-clock.c +2 -2
@@ -69,10 +69,10 @@ static ssize_t posix_clock_read(struct file *fp, char __user *buf,static unsigned int posix_clock_poll (struct file *fp,poll_table *wait) {struct posix_clock *clk =get_posix_clock (fp); -int result =0 ; +unsigned int result =0 ;if (!clk) -return -ENODEV ; +return POLLERR ;if (clk->ops.poll) result = clk->ops.poll (clk, fp, wait);
Signature · 1 line
2.1.4
tip-bot for Richard Cochran
merge bot[tip:timers/urgent] posix-clock: Fix return code on the poll method's error path
posix-clock: Fix return code on the poll method's error path
Commit-ID: 1b9f23727abb92c5e58f139e7d180befcaa06fe0 Gitweb: http://git.kernel.org/tip/1b9f23727abb92c5e58f139e7d180befcaa06fe0 Author: Richard Cochran <[email protected]> AuthorDate: Tue, 22 Dec 2015 22:19:58 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Tue, 29 Dec 2015 11:33:06 +0100
posix-clock: Fix return code on the poll method's error path
The posix_clock_poll function is supposed to return a bit mask of POLLxxx values. However, in case the hardware has disappeared (due to hot plugging for example) this code returns -ENODEV in a futile attempt to throw an error at the file descriptor level. The kernel's file_operations interface does not accept such error codes from the poll method. Instead, this function aught to return POLLERR.
The value -ENODEV does, in fact, contain the POLLERR bit (and almost all the other POLLxxx bits as well), but only by chance. This patch fixes code to return a proper bit mask.
Credit goes to Markus Elfring for pointing out the suspicious signed/unsigned mismatch.
--- kernel/time/posix-clock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/time/posix-clock.c +2 -2
@@ -69,10 +69,10 @@ static ssize_t posix_clock_read(struct file *fp, char __user *buf,static unsigned int posix_clock_poll (struct file *fp,poll_table *wait) {struct posix_clock *clk =get_posix_clock (fp); -int result =0 ; +unsigned int result =0 ;if (!clk) -return -ENODEV ; +return POLLERR ;if (clk->ops.poll) result = clk->ops.poll (clk, fp, wait);