uio.c: Fix warning: 'ret' might be used uninitialized

Fix warning: 'ret' might be used uninitialized

Signed-off-by Vitalii Demianets <[email protected]>

---
 drivers/uio/uio.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

drivers/uio/uio.c +12 -4

index 5110f36..c33fd18 100644
@@ -280,12 +280,16 @@ static int uio_dev_add_attributes(struct uio_device *idev)
 			map_found = 1;
 			idev->map_dir = kobject_create_and_add("maps",
 							&idev->dev->kobj);
-			if (!idev->map_dir)
+			if (!idev->map_dir) {
+				ret = -ENOMEM;
 				goto err_map;
+			}
 		}
 		map = kzalloc(sizeof(*map), GFP_KERNEL);
-		if (!map)
+		if (!map) {
+			ret = -ENOMEM;
 			goto err_map;
+		}
 		kobject_init(&map->kobj, &map_attr_type);
 		map->mem = mem;
 		mem->map = map;

@@ -305,12 +309,16 @@ static int uio_dev_add_attributes(struct uio_device *idev)
 			portio_found = 1;
 			idev->portio_dir = kobject_create_and_add("portio",
 							&idev->dev->kobj);
-			if (!idev->portio_dir)
+			if (!idev->portio_dir) {
+				ret = -ENOMEM;
 				goto err_portio;
+			}
 		}
 		portio = kzalloc(sizeof(*portio), GFP_KERNEL);
-		if (!portio)
+		if (!portio) {
+			ret = -ENOMEM;
 			goto err_portio;
+		}
 		kobject_init(&portio->kobj, &portio_attr_type);
 		portio->port = port;
 		port->portio = portio;

Signature · 1 line

1.7.8.6

From 228445996bb75a44d16b6237eca6a0916d9b2d7e Mon Sep 17 00:00:00 2001

Thanks, good catch, but why don't you simply do this:

Quoted 2 lines

From 228445996bb75a44d16b6237eca6a0916d9b2d7e Mon Sep 17 00:00:00 2001

From: "Hans J. Koch" <[email protected]>
Date: Tue, 27 Nov 2012 23:38:00 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/uio/uio.c +1 -1

index 5110f36..fc60e35 100644
@@ -263,7 +263,7 @@ static struct class uio_class = {
  */
 static int uio_dev_add_attributes(struct uio_device *idev)
 {
-	int ret;
+	int ret = -ENOMEM;
 	int mi, pi;
 	int map_found = 0;
 	int portio_found = 0;

Quoted 3 lines · On Wednesday 28 November 2012 00:43:41 Hans J. Koch wrote:

Thanks, good catch, but why don't you simply do this:

Just a matter of personal preference.

As a maintainer you can apply either
patch you want. I guess you would prefer your approach and I have no objections to that :)

Quoted 27 lines

Quoted 3 lines

From 228445996bb75a44d16b6237eca6a0916d9b2d7e Mon Sep 17 00:00:00 2001

From: "Hans J. Koch" <[email protected]>
Date: Tue, 27 Nov 2012 23:38:00 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/uio/uio.c +1 -1

index 5110f36..fc60e35 100644
@@ -263,7 +263,7 @@ static struct class uio_class = {
  */
 static int uio_dev_add_attributes(struct uio_device *idev)
 {
-	int ret;
+	int ret = -ENOMEM;
 	int mi, pi;
 	int map_found = 0;
 	int portio_found = 0;

Signature · 2 lines

With Best Regards,
Vitalii Demianets

Quoted 6 lines · Vitalii Demianets

Quoted 3 lines · On Wednesday 28 November 2012 00:43:41 Hans J. Koch wrote:

Thanks, good catch, but why don't you simply do this:

Just a matter of personal preference.

Your patch: 1 files changed, 12 insertions(+), 4 deletions(-) My patch: 1 files changed, 1 insertions(+), 1 deletions(-)

Both achieve exactly the same. That's not a matter of personal preference, that's the difference between a working solution and a good solution. In the kernel, we want the latter.

Quoted 3 lines

As a maintainer you can apply either
patch you want. I guess you would prefer your approach and I have no objections to that :)

That's not the right kind of comment. Don't make it a habit.

Thanks,
Hans

Quoted 25 lines

Quoted 25 lines

Quoted 3 lines

From 228445996bb75a44d16b6237eca6a0916d9b2d7e Mon Sep 17 00:00:00 2001

From: "Hans J. Koch" <[email protected]>
Date: Tue, 27 Nov 2012 23:38:00 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/uio/uio.c +1 -1

index 5110f36..fc60e35 100644
@@ -263,7 +263,7 @@ static struct class uio_class = {
  */
 static int uio_dev_add_attributes(struct uio_device *idev)
 {
-   int ret;
+   int ret = -ENOMEM;
    int mi, pi;

Quoted 10 lines

drivers/uio/uio.c static struct class uio_class = { · line 269

Quoted 2 lines

drivers/uio/uio.c +0 -0

    int map_found = 0;
    int portio_found = 0;

--
With Best Regards,
Vitalii Demianets

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

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Quoted 51 lines · Vitalii Demianets

Quoted 6 lines · On Wednesday 28 November 2012 00:43:41 Hans J. Koch wrote:

Quoted 3 lines

Thanks, good catch, but why don't you simply do this:

Just a matter of personal preference.

Your patch: 1 files changed, 12 insertions(+), 4 deletions(-) My patch: 1 files changed, 1 insertions(+), 1 deletions(-)

Both achieve exactly the same. That's not a matter of personal preference, that's the difference between a working solution and a good solution. In the kernel, we want the latter.

Quoted 3 lines

As a maintainer you can apply either
patch you want. I guess you would prefer your approach and I have no objections to that :)

That's not the right kind of comment. Don't make it a habit.

Thanks,
Hans

Quoted 27 lines

Quoted 27 lines

Quoted 3 lines

From 228445996bb75a44d16b6237eca6a0916d9b2d7e Mon Sep 17 00:00:00 2001

From: "Hans J. Koch" <[email protected]>
Date: Tue, 27 Nov 2012 23:38:00 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/uio/uio.c +1 -1

index 5110f36..fc60e35 100644
@@ -263,7 +263,7 @@ static struct class uio_class = {
  */
 static int uio_dev_add_attributes(struct uio_device *idev)
 {
-   int ret;
+   int ret = -ENOMEM;
    int mi, pi;
    int map_found = 0;
    int portio_found = 0;

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Quoted 4 lines

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Quoted 50 lines · Hans J. Koch

Quoted 49 lines · Vitalii Demianets

Quoted 4 lines · On Wednesday 28 November 2012 00:43:41 Hans J. Koch wrote:

Quoted 1 line

Thanks, good catch, but why don't you simply do this:

Just a matter of personal preference.

Your patch: 1 files changed, 12 insertions(+), 4 deletions(-) My patch: 1 files changed, 1 insertions(+), 1 deletions(-)

Both achieve exactly the same. That's not a matter of personal preference, that's the difference between a working solution and a good solution. In the kernel, we want the latter.

Quoted 3 lines

As a maintainer you can apply either
patch you want. I guess you would prefer your approach and I have no objections to that :)

That's not the right kind of comment. Don't make it a habit.

Thanks,
Hans

Quoted 27 lines

Quoted 27 lines

Quoted 2 lines

From 228445996bb75a44d16b6237eca6a0916d9b2d7e Mon Sep 17 00:00:00 2001

From: "Hans J. Koch" <[email protected]>
Date: Tue, 27 Nov 2012 23:38:00 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/uio/uio.c +1 -1

index 5110f36..fc60e35 100644
@@ -263,7 +263,7 @@ static struct class uio_class = {
  */
 static int uio_dev_add_attributes(struct uio_device *idev)
 {
-   int ret;
+   int ret = -ENOMEM;
    int mi, pi;
    int map_found = 0;
    int portio_found = 0;

Quoted 11 lines · On Thursday 29 November 2012 18:05:27 Tux9 wrote:

Quoted 4 lines

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Oops, disregard this. After looking at it more thoroughly I got your point. You are right, ret is overridden at first iteration (mi == 0), so Hans's approach does not work.
I must do more thinking before replying in a hurry.

From 00c3c734c0dde67873a628bcb18cee403c95c301 Mon Sep 17 00:00:00 2001

Quoted 12 lines · Vitalii Demianets

Quoted 11 lines · On Thursday 29 November 2012 18:05:27 Tux9 wrote:

Quoted 4 lines

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Quoted 4 lines

Oops, disregard this. After looking at it more thoroughly I got your point. You are right, ret is overridden at first iteration (mi == 0), so Hans's approach does not work.
I must do more thinking before replying in a hurry.

You're right. Initialization of "ret" has to take place at the beginning of the loop.

I think this version is right:

From 00c3c734c0dde67873a628bcb18cee403c95c301 Mon Sep 17 00:00:00 2001
From: "Hans J. Koch" <[email protected]>
Date: Fri, 30 Nov 2012 00:51:50 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/uio/uio.c +2 -1

index 5110f36..0c80df2 100644
@@ -273,6 +273,7 @@ static int uio_dev_add_attributes(struct uio_device *idev)
        struct uio_portio *portio;
 
        for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
+               ret = -ENOMEM;
                mem = &idev->info->mem[mi];
                if (mem->size == 0)
                        break;

@@ -298,6 +299,7 @@ static int uio_dev_add_attributes(struct uio_device *idev)
        }
 
        for (pi = 0; pi < MAX_UIO_PORT_REGIONS; pi++) {
+               ret = -ENOMEM;
                port = &idev->info->port[pi];
                if (port->size == 0)
                        break;
--

1.7.9

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

I like Vitalii's solution more. Hans's solution assign the value -ENOMEM to ret in every round of the loop, which is a kind of wasting CPU cycles.

Quoted 64 lines · Vitalii Demianets

Quoted 12 lines

Quoted 11 lines · On Thursday 29 November 2012 18:05:27 Tux9 wrote:

Quoted 4 lines

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Quoted 4 lines

Oops, disregard this. After looking at it more thoroughly I got your point. You are right, ret is overridden at first iteration (mi == 0), so Hans's approach does not work.
I must do more thinking before replying in a hurry.

You're right. Initialization of "ret" has to take place at the beginning of the loop.

I think this version is right:

From 00c3c734c0dde67873a628bcb18cee403c95c301 Mon Sep 17 00:00:00 2001
From: "Hans J. Koch" <[email protected]>
Date: Fri, 30 Nov 2012 00:51:50 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/uio/uio.c +2 -1

index 5110f36..0c80df2 100644
@@ -273,6 +273,7 @@ static int uio_dev_add_attributes(struct uio_device *idev)
        struct uio_portio *portio;
 
        for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
+               ret = -ENOMEM;
                mem = &idev->info->mem[mi];
                if (mem->size == 0)
                        break;

@@ -298,6 +299,7 @@ static int uio_dev_add_attributes(struct uio_device *idev)
        }
 
        for (pi = 0; pi < MAX_UIO_PORT_REGIONS; pi++) {
+               ret = -ENOMEM;
                port = &idev->info->port[pi];
                if (port->size == 0)
                        break;
--

1.7.9

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

Quoted 3 lines · On Fri, Nov 30, 2012 at 12:12:46PM +0100, Tux9 wrote:

I like Vitalii's solution more. Hans's solution assign the value -ENOMEM to ret in every round of the loop, which is a kind of wasting CPU cycles.

The difference between

1 files changed, 12 insertions(+), 4 deletions(-) and
1 files changed, 2 insertions(+), 0 deletions(-)

is more important. Note that this code is not in a fast path but only called once at device creation.

Quoted 62 lines · Hans J. Koch

Quoted 59 lines · Vitalii Demianets

Quoted 12 lines

Quoted 11 lines · On Thursday 29 November 2012 18:05:27 Tux9 wrote:

Quoted 4 lines

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Quoted 4 lines

Oops, disregard this. After looking at it more thoroughly I got your point. You are right, ret is overridden at first iteration (mi == 0), so Hans's approach does not work.
I must do more thinking before replying in a hurry.

You're right. Initialization of "ret" has to take place at the beginning of the loop.

I think this version is right:

From 00c3c734c0dde67873a628bcb18cee403c95c301 Mon Sep 17 00:00:00 2001
From: "Hans J. Koch" <[email protected]>
Date: Fri, 30 Nov 2012 00:51:50 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/uio/uio.c +2 -1

index 5110f36..0c80df2 100644
@@ -273,6 +273,7 @@ static int uio_dev_add_attributes(struct uio_device *idev)
        struct uio_portio *portio;
 
        for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
+               ret = -ENOMEM;
                mem = &idev->info->mem[mi];
                if (mem->size == 0)
                        break;

@@ -298,6 +299,7 @@ static int uio_dev_add_attributes(struct uio_device *idev)
        }
 
        for (pi = 0; pi < MAX_UIO_PORT_REGIONS; pi++) {
+               ret = -ENOMEM;
                port = &idev->info->port[pi];
                if (port->size == 0)
                        break;
--

1.7.9

Quoted 6 lines

Quoted 5 lines

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

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

Quoted 9 lines · Hans J. Koch

Quoted 3 lines · On Fri, Nov 30, 2012 at 12:12:46PM +0100, Tux9 wrote:

I like Vitalii's solution more. Hans's solution assign the value -ENOMEM to ret in every round of the loop, which is a kind of wasting CPU cycles.

The difference between

1 files changed, 12 insertions(+), 4 deletions(-) and
1 files changed, 2 insertions(+), 0 deletions(-)

Quoted 2 lines

is more important. Note that this code is not in a fast path but only called once at device creation.

Why do you think the size of the patch is so important? I think the most important thing is the coding style and efficiency. I agree efficiency is not important in this case, but what about coding style?

Your code is _not_ very easy to understand.

It's a very natural thing
to set the return value and then goto the error-handling codes, which is exactly same as what Vitalii did, rather than setting an initial value in the beginning of each round of the loop as you did.

There are
also a bunch of codes in kernel in the same style with Vitalii, but I cannot find anything same as your codes.

Quoted 10 lines · Cong Ding

Quoted 9 lines · Hans J. Koch

Quoted 3 lines · On Fri, Nov 30, 2012 at 12:12:46PM +0100, Tux9 wrote:

I like Vitalii's solution more. Hans's solution assign the value -ENOMEM to ret in every round of the loop, which is a kind of wasting CPU cycles.

The difference between

1 files changed, 12 insertions(+), 4 deletions(-) and
1 files changed, 2 insertions(+), 0 deletions(-)

Quoted 5 lines

Quoted 2 lines

is more important. Note that this code is not in a fast path but only called once at device creation.

Why do you think the size of the patch is so important? I think the most important thing is the coding style and efficiency. I agree efficiency is not important in this case, but what about coding style?

Most important. Short is beautiful.

Quoted 1 line

Your code is _not_ very easy to understand.

You think so?

Quoted 4 lines

It's a very natural thing
to set the return value and then goto the error-handling codes, which is exactly same as what Vitalii did, rather than setting an initial value in the beginning of each round of the loop as you did.

Setting a default value at the beginning of a block is the most natural thing. I don't want to repeat the same code in three places.

Quoted 3 lines

There are
also a bunch of codes in kernel in the same style with Vitalii, but I cannot find anything same as your codes.

If you follow LKML closely, you'll notice that simplifying code by replacing unnecessary repetitions with shorter versions is always welcome. If we didn't go for that, the kernel source would be a few million lines bigger by now.

Thanks,
Hans

If it is really necessary to save the 4 lines of codes, I would suggest to do in the following style. But you are more senior than me, so I may be wrong in this aspect.

Quoted 11 lines · Vitalii Demianets

Quoted 10 lines · On Thursday 29 November 2012 18:05:27 Tux9 wrote:

Quoted 4 lines

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Quoted 4 lines

Oops, disregard this. After looking at it more thoroughly I got your point. You are right, ret is overridden at first iteration (mi == 0), so Hans's approach does not work.
I must do more thinking before replying in a hurry.

You're right. Initialization of "ret" has to take place at the beginning of the loop.

I think this version is right:

Quoted 1 line

From 00c3c734c0dde67873a628bcb18cee403c95c301 Mon Sep 17 00:00:00 2001

From: "Hans J. Koch" <[email protected]>
Date: Fri, 30 Nov 2012 00:51:50 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/uio/uio.c +2 -0

index 5110f36..0c80df2 100644
@@ -273,6 +273,7 @@ static int uio_dev_add_attributes(struct uio_device
 *idev) struct uio_portio *portio;
 
 	for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
+		ret = -ENOMEM;
 		mem = &idev->info->mem[mi];
 		if (mem->size == 0)
 			break;

@@ -298,6 +299,7 @@ static int uio_dev_add_attributes(struct uio_device
 *idev) }
 
 	for (pi = 0; pi < MAX_UIO_PORT_REGIONS; pi++) {
+		ret = -ENOMEM;
 		port = &idev->info->port[pi];
 		if (port->size == 0)
 			break;

Quoted 17 lines · On Friday 30 November 2012 01:58:22 Hans J. Koch wrote:

Quoted 15 lines · Vitalii Demianets

Quoted 10 lines · On Thursday 29 November 2012 18:05:27 Tux9 wrote:

Quoted 4 lines

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Oops, disregard this. After looking at it more thoroughly I got your point. You are right, ret is overridden at first iteration (mi == 0), so Hans's approach does not work.
I must do more thinking before replying in a hurry.

Quoted 4 lines

You're right. Initialization of "ret" has to take place at the beginning of the loop.

I think this version is right:

Yes, this looks right for me.

Quoted 34 lines

Quoted 2 lines

From 00c3c734c0dde67873a628bcb18cee403c95c301 Mon Sep 17 00:00:00 2001

From: "Hans J. Koch" <[email protected]>
Date: Fri, 30 Nov 2012 00:51:50 +0100
Subject: [PATCH] uio: Fix warning: 'ret' might be used uninitialized

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

---
 drivers/uio/uio.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

drivers/uio/uio.c +2 -0

index 5110f36..0c80df2 100644
@@ -273,6 +273,7 @@ static int uio_dev_add_attributes(struct uio_device
 *idev) struct uio_portio *portio;
 
 	for (mi = 0; mi < MAX_UIO_MAPS; mi++) {
+		ret = -ENOMEM;
 		mem = &idev->info->mem[mi];
 		if (mem->size == 0)
 			break;

@@ -298,6 +299,7 @@ static int uio_dev_add_attributes(struct uio_device
 *idev) }
 
 	for (pi = 0; pi < MAX_UIO_PORT_REGIONS; pi++) {
+		ret = -ENOMEM;
 		port = &idev->info->port[pi];
 		if (port->size == 0)
 			break;

Quoted 21 lines · Vitalii Demianets

Quoted 20 lines · On Friday 30 November 2012 01:58:22 Hans J. Koch wrote:

Quoted 11 lines · Vitalii Demianets

Quoted 10 lines · On Thursday 29 November 2012 18:05:27 Tux9 wrote:

Quoted 4 lines

Hans, I think there are something wrong in your patch, while Vitalii's is right. The variable "ret" is reused in line 292 and line 295, so the value of "ret" would be overridden (if it goto err_map in line 284 when mi>=1).

Actually, both patches do exactly the same thing. Hans's patch establishes default value for the ret for all those "other" cases when ret is not explicitly overridden. My patch explicitly enumerates all those "other" cases in more wordily manner.

Quoted 4 lines

Oops, disregard this. After looking at it more thoroughly I got your point. You are right, ret is overridden at first iteration (mi == 0), so Hans's approach does not work.
I must do more thinking before replying in a hurry.

You're right. Initialization of "ret" has to take place at the beginning of the loop.

Quoted 3 lines

Quoted 1 line

I think this version is right:

Yes, this looks right for me.

OK, I'll send that patch offically, then. This might also be material for the stable updates. Greg?

In two cases, the return value variable "ret" can be undefined.

Signed-off-by Hans J. Koch <[email protected]>

Reported-by Vitalii Demianets <[email protected]>

Thanks a lot for reporting and discussing that problem. I'll add a

Reported-by Vitalii Demianets <[email protected]>

if you have no objections.

Quoted 5 lines

Thanks a lot for reporting and discussing that problem. I'll add a

Reported-by Vitalii Demianets <[email protected]>

if you have no objections.

No objections. Thanks, Hans.

Quoted 8 lines · On Friday 30 November 2012 23:39:06 Hans J. Koch wrote:

Quoted 5 lines

Thanks a lot for reporting and discussing that problem. I'll add a

Reported-by Vitalii Demianets <[email protected]>

if you have no objections.

No objections. Thanks, Hans.

By the way, what do you think about my revised version of uio_pdrv_genirq.c patch from November 29th?