GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

PATCH New Sachin Kamat 3 participants 8 messages Jul 15, 2013 – Jul 22, 2013 GFS2 file system

lore

Share & tools

lore

Sachin Kamat

author

2013-07-15 11:28 UTC #0 lore raw

PATCH 1/1 GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.

Signed-off-by Sachin Kamat <[email protected]>

---
Compile tested and based on the following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)

Dependent on [1]
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html

---
 fs/gfs2/inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/gfs2/inode.c +2 -1

index bbb2715..a186ebd 100644
@@ -19,6 +19,7 @@
 #include <linux/crc32.h>
 #include <linux/fiemap.h>
 #include <linux/security.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
 #include "gfs2.h"

@@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		}
 		gfs2_glock_dq_uninit(ghs);
 		if (IS_ERR(d))
-			return PTR_RET(d);
+			return PTR_ERR_OR_ZERO(d);
 		return error;
 	} else if (error != -ENOENT) {
 		goto fail_gunlock;

Signature · 1 line

1.7.9.5

Steven Whitehouse

2013-07-15 17:18 UTC #1 lore raw

PATCH 1/1 GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

Hi,

Quoted 26 lines · Sachin Kamat

PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
Signed-off-by Sachin Kamat <[email protected]>
---
Compile tested and based on the following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
Dependent on [1]
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
---
 fs/gfs2/inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/gfs2/inode.c +1 -0
index bbb2715..a186ebd 100644
@@ -19,6 +19,7 @@
 #include <linux/crc32.h>
 #include <linux/fiemap.h>
 #include <linux/security.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
 #include "gfs2.h"

fs/gfs2/inode.c static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, · line 598

Quoted 5 lines

fs/gfs2/inode.c +1 -1

@@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		}
 		gfs2_glock_dq_uninit(ghs);
 		if (IS_ERR(d))
-			return PTR_RET(d);
+			return PTR_ERR_OR_ZERO(d);

I'm not sure I follow what this is supposed to be doing... what is the reason for this change? This macro/function doesn't seem to be defined in the current kernel, so I assume that it is "coming soon" but the thread pointed to above wasn't very enlightening,

Steve.

Quoted 3 lines

fs/gfs2/inode.c +0 -0

 		return error;
 	} else if (error != -ENOENT) {
 		goto fail_gunlock;

Rusty Russell

2013-07-16 06:35 UTC #2 lore raw

PATCH 1/1 GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

Quoted 27 lines · Steven Whitehouse

Hi,
Quoted 24 lines · Sachin Kamat
PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
Signed-off-by Sachin Kamat <[email protected]>
---
Compile tested and based on the following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
Dependent on [1]
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
---
 fs/gfs2/inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/gfs2/inode.c +1 -0
index bbb2715..a186ebd 100644
@@ -19,6 +19,7 @@
 #include <linux/crc32.h>
 #include <linux/fiemap.h>
 #include <linux/security.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 

Quoted 12 lines

fs/gfs2/inode.c static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, · line 598
Quoted 7 lines
fs/gfs2/inode.c +1 -1
 #include "gfs2.h"
@@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		}
 		gfs2_glock_dq_uninit(ghs);
 		if (IS_ERR(d))
-			return PTR_RET(d);
+			return PTR_ERR_OR_ZERO(d);
I'm not sure I follow what this is supposed to be doing... what is the reason for this change? This macro/function doesn't seem to be defined in the current kernel, so I assume that it is "coming soon" but the thread pointed to above wasn't very enlightening,

It's a clarification rename.

But this fix is wrong, it should just be changed to PTR_ERR(d). It never needed PTR_RET() in the first place.

Thanks,
Rusty.

Steven Whitehouse

2013-07-17 07:41 UTC #3 lore raw

PATCH 1/1 GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

Hi,

Quoted 41 lines · Rusty Russell

Quoted 27 lines · Steven Whitehouse
Hi,
Quoted 24 lines · Sachin Kamat
PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
Signed-off-by Sachin Kamat <[email protected]>
---
Compile tested and based on the following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
Dependent on [1]
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
---
 fs/gfs2/inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/gfs2/inode.c +1 -0
index bbb2715..a186ebd 100644
@@ -19,6 +19,7 @@
 #include <linux/crc32.h>
 #include <linux/fiemap.h>
 #include <linux/security.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
Quoted 12 lines
fs/gfs2/inode.c static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, · line 598
Quoted 7 lines
fs/gfs2/inode.c +1 -1
 #include "gfs2.h"
@@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		}
 		gfs2_glock_dq_uninit(ghs);
 		if (IS_ERR(d))
-			return PTR_RET(d);
+			return PTR_ERR_OR_ZERO(d);
I'm not sure I follow what this is supposed to be doing... what is the reason for this change? This macro/function doesn't seem to be defined in the current kernel, so I assume that it is "coming soon" but the thread pointed to above wasn't very enlightening,

Quoted 7 lines

It's a clarification rename.
But this fix is wrong, it should just be changed to PTR_ERR(d). It never needed PTR_RET() in the first place.
Thanks,
Rusty.

Ok, thanks for clarifying. I've sorted out a patch, attached below, which I'll put in my tree unless there are any objections. I don't think that it should affect your patch series, but let me know if there is a problem,

Steve.

From 1067f2a5b96d11c2c3dd5ba83e3969cc5ed51b50 Mon Sep 17 00:00:00 2001
From: Steven Whitehouse <[email protected]>
Date: Wed, 17 Jul 2013 08:11:32 +0100
Subject: [PATCH] GFS2: Fix typo in gfs2_create_inode()

PTR_RET should be PTR_ERR

Reported-by Sachin Kamat <[email protected]>

Cc Rusty Russell <[email protected]>

Signed-off-by Steven Whitehouse <[email protected]>

fs/gfs2/inode.c +1 -1

index bbb2715..a01b8fd 100644
@@ -594,7 +594,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		}
 		gfs2_glock_dq_uninit(ghs);
 		if (IS_ERR(d))
-			return PTR_RET(d);
+			return PTR_ERR(d);
 		return error;
 	} else if (error != -ENOENT) {
 		goto fail_gunlock;

Signature · 1 line

1.7.4

Rusty Russell

Applied

2013-07-22 01:42 UTC #4 lore raw

PATCH 1/1 GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

Quoted 85 lines · Steven Whitehouse

Hi,
Quoted 41 lines · Rusty Russell
Quoted 27 lines · Steven Whitehouse
Hi,
Quoted 24 lines · Sachin Kamat
PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
Signed-off-by Sachin Kamat <[email protected]>
---
Compile tested and based on the following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
Dependent on [1]
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
---
 fs/gfs2/inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/gfs2/inode.c +1 -0
index bbb2715..a186ebd 100644
@@ -19,6 +19,7 @@
 #include <linux/crc32.h>
 #include <linux/fiemap.h>
 #include <linux/security.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
Quoted 12 lines
fs/gfs2/inode.c static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, · line 598
Quoted 7 lines
fs/gfs2/inode.c +1 -1
 #include "gfs2.h"
@@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		}
 		gfs2_glock_dq_uninit(ghs);
 		if (IS_ERR(d))
-			return PTR_RET(d);
+			return PTR_ERR_OR_ZERO(d);
I'm not sure I follow what this is supposed to be doing... what is the reason for this change? This macro/function doesn't seem to be defined in the current kernel, so I assume that it is "coming soon" but the thread pointed to above wasn't very enlightening,
Quoted 7 lines
It's a clarification rename.
But this fix is wrong, it should just be changed to PTR_ERR(d). It never needed PTR_RET() in the first place.
Thanks,
Rusty.
Ok, thanks for clarifying. I've sorted out a patch, attached below, which I'll put in my tree unless there are any objections. I don't think that it should affect your patch series, but let me know if there is a problem,
Steve.
Quoted 1 line
From 1067f2a5b96d11c2c3dd5ba83e3969cc5ed51b50 Mon Sep 17 00:00:00 2001
From: Steven Whitehouse <[email protected]>
Date: Wed, 17 Jul 2013 08:11:32 +0100
Subject: [PATCH] GFS2: Fix typo in gfs2_create_inode()
PTR_RET should be PTR_ERR
Reported-by Sachin Kamat <[email protected]>
Cc Rusty Russell <[email protected]>
Signed-off-by Steven Whitehouse <[email protected]>
fs/gfs2/inode.c +1 -1
index bbb2715..a01b8fd 100644
@@ -594,7 +594,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		}
 		gfs2_glock_dq_uninit(ghs);
 		if (IS_ERR(d))
-			return PTR_RET(d);
+			return PTR_ERR(d);
 		return error;
 	} else if (error != -ENOENT) {
 		goto fail_gunlock;
Signature · 1 line
1.7.4

Thanks, applied to my PTR_RET branch, but you could just put it in your tree too.

Cheers,
Rusty.

Sachin Kamat

author

2013-07-16 08:28 UTC #5 lore raw

PATCH 1/1 GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

In reply to Steven Whitehouse

Quoted 27 lines · Steven Whitehouse

Hi,
Quoted 24 lines · Sachin Kamat
PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
Signed-off-by Sachin Kamat <[email protected]>
---
Compile tested and based on the following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
Dependent on [1]
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
---
 fs/gfs2/inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/gfs2/inode.c +1 -0
index bbb2715..a186ebd 100644
@@ -19,6 +19,7 @@
 #include <linux/crc32.h>
 #include <linux/fiemap.h>
 #include <linux/security.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 

Quoted 10 lines

fs/gfs2/inode.c static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, · line 598
Quoted 7 lines
fs/gfs2/inode.c +1 -1
 #include "gfs2.h"
@@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
              }
              gfs2_glock_dq_uninit(ghs);
              if (IS_ERR(d))
-                     return PTR_RET(d);
+                     return PTR_ERR_OR_ZERO(d);
I'm not sure I follow what this is supposed to be doing... what is the reason for this change?

As far as I understand there is no change in the functionality of the code. The function has been renamed to better
reflect its functionality.

Quoted 3 lines

This macro/function doesn't seem to be defined
in the current kernel, so I assume that it is "coming soon" but the thread pointed to above wasn't very enlightening,

Yes, this is currently available in Rusty's tree [1] and now merged into linux-next. Since he has done this replacement in most of the places, i guess he
will pick this patch too with your ack. Rusty?

[1] git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git
(branch: PTR_RET)

Signature · 2 lines

With warm regards,
Sachin

Steven Whitehouse

2013-07-16 09:06 UTC #6 lore raw

PATCH 1/1 GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

Hi,

Quoted 39 lines · Sachin Kamat

Quoted 27 lines · Steven Whitehouse
Hi,
Quoted 24 lines · Sachin Kamat
PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
Signed-off-by Sachin Kamat <[email protected]>
---
Compile tested and based on the following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
Dependent on [1]
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
---
 fs/gfs2/inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/gfs2/inode.c +1 -0
index bbb2715..a186ebd 100644
@@ -19,6 +19,7 @@
 #include <linux/crc32.h>
 #include <linux/fiemap.h>
 #include <linux/security.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
Quoted 10 lines
fs/gfs2/inode.c static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, · line 598
Quoted 7 lines
fs/gfs2/inode.c +1 -1
 #include "gfs2.h"
@@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
              }
              gfs2_glock_dq_uninit(ghs);
              if (IS_ERR(d))
-                     return PTR_RET(d);
+                     return PTR_ERR_OR_ZERO(d);
I'm not sure I follow what this is supposed to be doing... what is the reason for this change?

Quoted 4 lines

As far as I understand there is no change in the functionality of the code. The function has been renamed to better
reflect its functionality.

I think that the problem is that PTR_RET is the wrong function in the first place. It should be PTR_ERR rather than PTR_RET, since we already know that the value is an error value,

Steve.

Quoted 12 lines

Quoted 3 lines
This macro/function doesn't seem to be defined
in the current kernel, so I assume that it is "coming soon" but the thread pointed to above wasn't very enlightening,
Yes, this is currently available in Rusty's tree [1] and now merged into linux-next. Since he has done this replacement in most of the places, i guess he
will pick this patch too with your ack. Rusty?
[1] git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git
(branch: PTR_RET)

Sachin Kamat

author

2013-07-16 09:25 UTC #7 lore raw

PATCH 1/1 GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

Quoted 42 lines · Steven Whitehouse

Hi,
Quoted 39 lines · Sachin Kamat
Quoted 37 lines · Steven Whitehouse
Hi,
Quoted 26 lines · Sachin Kamat
PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
Signed-off-by Sachin Kamat <[email protected]>
---
Compile tested and based on the following tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git (PTR_RET)
Dependent on [1]
[1] http://lkml.indiana.edu/hypermail/linux/kernel/1306.2/00010.html
---
 fs/gfs2/inode.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/gfs2/inode.c +1 -0
index bbb2715..a186ebd 100644
@@ -19,6 +19,7 @@
 #include <linux/crc32.h>
 #include <linux/fiemap.h>
 #include <linux/security.h>
+#include <linux/err.h>
 #include <asm/uaccess.h>
 
 #include "gfs2.h"
fs/gfs2/inode.c static int gfs2_create_inode(struct inode *dir, struct dentry *dentry, · line 598
Quoted 5 lines
fs/gfs2/inode.c +1 -1
@@ -594,7 +595,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
              }
              gfs2_glock_dq_uninit(ghs);
              if (IS_ERR(d))
-                     return PTR_RET(d);
+                     return PTR_ERR_OR_ZERO(d);
I'm not sure I follow what this is supposed to be doing... what is the reason for this change?

Quoted 7 lines

Quoted 4 lines
As far as I understand there is no change in the functionality of the code. The function has been renamed to better
reflect its functionality.
I think that the problem is that PTR_RET is the wrong function in the first place. It should be PTR_ERR rather than PTR_RET, since we already know that the value is an error value,

Yes you are right. I somehow overlooked this. It should just be PTR_ERR(). Please ignore this patch.

Signature · 2 lines

With warm regards,
Sachin