Skip to contentSkip to conversation

dm-log: fix return value in create_log_context function

PATCH In review Takahiro Yasui 2 participants 5 messages Dec 4, 2008 – Jan 8, 2009 Device mapper

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 dm-log: fix return value in create_log_context function

Hi,

This is very trivial fix, but let me post it to improve code readability. In the current implementation, caller function does not use the return value and no action is changed.

Regards,
---
Takahiro Yasui
Hitachi Computer Products (America) Inc.

When dm_io_client_create function fails in create_log_context function, return value is set to variable r but it is not used. r should be returned.

---
 drivers/md/dm-log.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/md/dm-log.c +1 -1
Index: linux-2.6.28-rc7/drivers/md/dm-log.c
===================================================================
@@ -461,7 +461,7 @@ static int create_log_context(struct dm_
 			r = PTR_ERR(lc->io_req.client);
 			DMWARN("couldn't allocate disk io client");
 			kfree(lc);
-			return -ENOMEM;
+			return r;
 		}
 
 		lc->disk_header = vmalloc(buf_size);
#1 lore raw

Hi all,

What do you think about this patch?

Quoted 1 line

DMWARN("couldn't allocate disk io client");

This message is not necessarily related to ENOMEM and returning variable "r" is better than doing -ENOMEM in terms of function encapsulation.

I appreciate any comments.

Regards,
Taka

Quoted 41 lines

Hi,

This is very trivial fix, but let me post it to improve code readability. In the current implementation, caller function does not use the return value and no action is changed.

Regards,
---
Takahiro Yasui
Hitachi Computer Products (America) Inc.

When dm_io_client_create function fails in create_log_context function, return value is set to variable r but it is not used. r should be returned.

---
 drivers/md/dm-log.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.28-rc7/drivers/md/dm-log.c
===================================================================

drivers/md/dm-log.c +1 -2
@@ -461,7 +461,7 @@ static int create_log_context(struct dm_
 			r = PTR_ERR(lc->io_req.client);
 			DMWARN("couldn't allocate disk io client");
 			kfree(lc);
-			return -ENOMEM;
+			return r;
 		}
 
 		lc->disk_header = vmalloc(buf_size);
 
--

dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

#2 lore raw

We might also want to change the message in that case then too, right? Maybe "Unable to create disk io client"?

brassow

Quoted 62 lines · Takahiro Yasui

Hi all,

What do you think about this patch?

Quoted 1 line

DMWARN("couldn't allocate disk io client");

This message is not necessarily related to ENOMEM and returning variable "r" is better than doing -ENOMEM in terms of function encapsulation.

I appreciate any comments.

Regards,
Taka

Quoted 38 lines

Hi,

This is very trivial fix, but let me post it to improve code readability. In the current implementation, caller function does not use the return value and no action is changed.

Regards,
---
Takahiro Yasui
Hitachi Computer Products (America) Inc.

When dm_io_client_create function fails in create_log_context function, return value is set to variable r but it is not used. r should be returned.

---
drivers/md/dm-log.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6.28-rc7/drivers/md/dm-log.c
===================================================================

drivers/md/dm-log.c +1 -2
@@ -461,7 +461,7 @@ static int create_log_context(struct dm_
 			r = PTR_ERR(lc->io_req.client);
 			DMWARN("couldn't allocate disk io client");
 			kfree(lc);
-			return -ENOMEM;
+			return r;
 		}
 
 		lc->disk_header = vmalloc(buf_size);
 
--
Quoted 3 lines

dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

#3 lore raw

Hi Jon,

Thanks for reviewing this patch.

Quoted 2 lines

We might also want to change the message in that case then too, right? Maybe "Unable to create disk io client"?

Thanks, "create disk io client" is better. But I changed as "couldn't create disk io client" for consistency with other messages.

Regards,
---
Takahiro Yasui
Hitachi Computer Products (America) Inc.

When dm_io_client_create function fails in create_log_context function, return value is set to r but it is not used. r should be returned. In addition, the warning message is properly corrected so that it doesn't indicate memory allocation.

---
 drivers/md/dm-log.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/md/dm-log.c +2 -2
Index: linux-2.6.28-rc8/drivers/md/dm-log.c
===================================================================
@@ -459,9 +459,9 @@ static int create_log_context(struct dm_
 								   PAGE_SIZE));
 		if (IS_ERR(lc->io_req.client)) {
 			r = PTR_ERR(lc->io_req.client);
-			DMWARN("couldn't allocate disk io client");
+			DMWARN("couldn't create disk io client");
 			kfree(lc);
-			return -ENOMEM;
+			return r;
 		}
 
 		lc->disk_header = vmalloc(buf_size);
#4 lore raw

ACK.

brassow

Quoted 50 lines · Takahiro Yasui

Hi Jon,

Thanks for reviewing this patch.

Quoted 2 lines

We might also want to change the message in that case then too, right? Maybe "Unable to create disk io client"?

Thanks, "create disk io client" is better. But I changed as "couldn't create disk io client" for consistency with other messages.

Regards,
---
Takahiro Yasui
Hitachi Computer Products (America) Inc.

When dm_io_client_create function fails in create_log_context function, return value is set to r but it is not used. r should be returned. In addition, the warning message is properly corrected so that it doesn't indicate memory allocation.

---
drivers/md/dm-log.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.28-rc8/drivers/md/dm-log.c
===================================================================

drivers/md/dm-log.c +2 -3
@@ -459,9 +459,9 @@ static int create_log_context(struct dm_
 								   PAGE_SIZE));
 		if (IS_ERR(lc->io_req.client)) {
 			r = PTR_ERR(lc->io_req.client);
-			DMWARN("couldn't allocate disk io client");
+			DMWARN("couldn't create disk io client");
 			kfree(lc);
-			return -ENOMEM;
+			return r;
 		}
 
 		lc->disk_header = vmalloc(buf_size);
 
--

dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel