Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1295539 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] posix-clock: Use an unsigned data type for a variable SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-20 09:30 +0100
    Re: [PATCH] posix-clock: Use an unsigned data type for a variable Julia Lawall <julia.lawall@lip6.fr> - 2015-12-20 12:20 +0100
      Re: [PATCH] posix-clock: Use an unsigned data type for a variable SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-20 13:50 +0100
        Re: [PATCH] posix-clock: Use an unsigned data type for a variable Julia Lawall <julia.lawall@lip6.fr> - 2015-12-20 14:00 +0100
          Re: [PATCH] posix-clock: Use an unsigned data type for a variable SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-20 14:10 +0100
    Re: [PATCH] posix-clock: Use an unsigned data type for a variable Richard Cochran <richardcochran@gmail.com> - 2015-12-20 21:20 +0100
      Re: posix-clock: Use an unsigned data type for a variable SF Markus Elfring <elfring@users.sourceforge.net> - 2015-12-21 10:40 +0100
        Re: posix-clock: Use an unsigned data type for a variable Richard Cochran <richardcochran@gmail.com> - 2015-12-21 11:20 +0100
    [PATCH] posix-clock: fix return code on the poll method's error path Richard Cochran <richardcochran@gmail.com> - 2015-12-22 22:30 +0100
      [tip:timers/urgent] posix-clock:   Fix return code on the poll method's error path tip-bot for Richard Cochran <tipbot@zytor.com> - 2015-12-29 11:40 +0100

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1295560

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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295583

>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295586

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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295590

>>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1295673

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1297068 — [PATCH] 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.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1298935 — [tip:timers/urgent] 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 <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);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web