[PATCH] posix-clock: Use an unsigned data type for a variable

[PATCH] posix-clock: Use an unsigned data type for a variable

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
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.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/time/posix-clock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index ce033c7..ac0b733 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -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;
-- 
2.6.3

[PATCH] posix-clock: Use an unsigned data type for a variable

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
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.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 kernel/time/posix-clock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index ce033c7..ac0b733 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -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;
-- 
2.6.3

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[Julia Lawall]

On Sun, 20 Dec 2015, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> 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.

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

> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  kernel/time/posix-clock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
> index ce033c7..ac0b733 100644
> --- a/kernel/time/posix-clock.c
> +++ b/kernel/time/posix-clock.c
> @@ -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;
> -- 
> 2.6.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[Julia Lawall]

On Sun, 20 Dec 2015, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> 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.

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

> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  kernel/time/posix-clock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
> index ce033c7..ac0b733 100644
> --- a/kernel/time/posix-clock.c
> +++ b/kernel/time/posix-clock.c
> @@ -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;
> -- 
> 2.6.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[SF Markus Elfring]

>> 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?


> 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

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[SF Markus Elfring]

>> 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?


> 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

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[Julia Lawall]

On Sun, 20 Dec 2015, SF Markus Elfring wrote:

> >> 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.

> > 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

> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[Julia Lawall]

On Sun, 20 Dec 2015, SF Markus Elfring wrote:

> >> 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.

> > 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

> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[SF Markus Elfring]

>>>> 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?


> 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?


>>> 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

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[SF Markus Elfring]

>>>> 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?


> 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?


>>> 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

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[Richard Cochran]

On Sun, Dec 20, 2015 at 09:19:20AM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> 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.

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

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

[Richard Cochran]

On Sun, Dec 20, 2015 at 09:19:20AM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> 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.

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

Re: posix-clock: Use an unsigned data type for a variable

[SF Markus Elfring]

> 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

Re: posix-clock: Use an unsigned data type for a variable

[SF Markus Elfring]

> 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

Re: posix-clock: Use an unsigned data type for a variable

[Richard Cochran]

On Mon, Dec 21, 2015 at 10:30:27AM +0100, SF Markus Elfring wrote:
> > 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

Re: posix-clock: Use an unsigned data type for a variable

[Richard Cochran]

On Mon, Dec 21, 2015 at 10:30:27AM +0100, SF Markus Elfring wrote:
> > 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] posix-clock: fix return code on the poll method's error path

[Richard Cochran]

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.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 kernel/time/posix-clock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index ce033c7..9cff0ab 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -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);
-- 
2.1.4

[PATCH] posix-clock: fix return code on the poll method's error path

[Richard Cochran]

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.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 kernel/time/posix-clock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index ce033c7..9cff0ab 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -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);
-- 
2.1.4

[tip:timers/urgent] posix-clock: Fix return code on the poll method's error path

[tip-bot for Richard Cochran]

Commit-ID:  1b9f23727abb92c5e58f139e7d180befcaa06fe0
Gitweb:     http://git.kernel.org/tip/1b9f23727abb92c5e58f139e7d180befcaa06fe0
Author:     Richard Cochran <richardcochran@gmail.com>
AuthorDate: Tue, 22 Dec 2015 22:19:58 +0100
Committer:  Thomas Gleixner <tglx@linutronix.de>
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.

Reported-by: Markus Elfring <elfring@users.sourceforge.net>
igned-off-by: Richard Cochran <richardcochran@gmail.com>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Julia Lawall <julia.lawall@lip6.fr>
Link: http://lkml.kernel.org/r/1450819198-17420-1-git-send-email-richardcochran@gmail.com
Cc: stable@vger.kernel.org
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/posix-clock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index ce033c7..9cff0ab 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -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);