[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Sachin Kamat]

PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.

Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
---
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(-)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index bbb2715..a186ebd 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -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;
-- 
1.7.9.5

[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Steven Whitehouse]

Hi,

On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
> 
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> ---
> 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(-)
> 
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index bbb2715..a186ebd 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -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);

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.

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

[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Rusty Russell]

Steven Whitehouse <swhiteho@redhat.com> writes:
> Hi,
>
> On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
>> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
>> 
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>> 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(-)
>> 
>> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
>> index bbb2715..a186ebd 100644
>> --- a/fs/gfs2/inode.c
>> +++ b/fs/gfs2/inode.c
>> @@ -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);
>
> 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.

[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Sachin Kamat]

On 15 July 2013 22:48, Steven Whitehouse <swhiteho@redhat.com> wrote:
> Hi,
>
> On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
>> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
>>
>> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> ---
>> 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(-)
>>
>> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
>> index bbb2715..a186ebd 100644
>> --- a/fs/gfs2/inode.c
>> +++ b/fs/gfs2/inode.c
>> @@ -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);
>
> 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.

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

-- 
With warm regards,
Sachin

[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Steven Whitehouse]

Hi,

On Tue, 2013-07-16 at 13:58 +0530, Sachin Kamat wrote:
> On 15 July 2013 22:48, Steven Whitehouse <swhiteho@redhat.com> wrote:
> > Hi,
> >
> > On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
> >> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
> >>
> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> >> ---
> >> 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(-)
> >>
> >> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> >> index bbb2715..a186ebd 100644
> >> --- a/fs/gfs2/inode.c
> >> +++ b/fs/gfs2/inode.c
> >> @@ -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);
> >
> > 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.
> 
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.

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

[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Sachin Kamat]

On 16 July 2013 14:36, Steven Whitehouse <swhiteho@redhat.com> wrote:
> Hi,
>
> On Tue, 2013-07-16 at 13:58 +0530, Sachin Kamat wrote:
>> On 15 July 2013 22:48, Steven Whitehouse <swhiteho@redhat.com> wrote:
>> > Hi,
>> >
>> > On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
>> >> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
>> >>
>> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> >> ---
>> >> 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(-)
>> >>
>> >> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
>> >> index bbb2715..a186ebd 100644
>> >> --- a/fs/gfs2/inode.c
>> >> +++ b/fs/gfs2/inode.c
>> >> @@ -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);
>> >
>> > 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.
>>
> 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.

-- 
With warm regards,
Sachin

[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Steven Whitehouse]

Hi,

On Tue, 2013-07-16 at 16:05 +0930, Rusty Russell wrote:
> Steven Whitehouse <swhiteho@redhat.com> writes:
> > Hi,
> >
> > On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
> >> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
> >> 
> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> >> ---
> >> 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(-)
> >> 
> >> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> >> index bbb2715..a186ebd 100644
> >> --- a/fs/gfs2/inode.c
> >> +++ b/fs/gfs2/inode.c
> >> @@ -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);
> >
> > 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.

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 <swhiteho@redhat.com>
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 <sachin.kamat@linaro.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index bbb2715..a01b8fd 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -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;
-- 
1.7.4

[Cluster-devel] [PATCH 1/1] GFS2: Replace PTR_RET with PTR_ERR_OR_ZERO

[Rusty Russell]

Steven Whitehouse <swhiteho@redhat.com> writes:
> Hi,
>
> On Tue, 2013-07-16 at 16:05 +0930, Rusty Russell wrote:
>> Steven Whitehouse <swhiteho@redhat.com> writes:
>> > Hi,
>> >
>> > On Mon, 2013-07-15 at 16:58 +0530, Sachin Kamat wrote:
>> >> PTR_RET is now deprecated. Use PTR_ERR_OR_ZERO instead.
>> >> 
>> >> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
>> >> ---
>> >> 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(-)
>> >> 
>> >> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
>> >> index bbb2715..a186ebd 100644
>> >> --- a/fs/gfs2/inode.c
>> >> +++ b/fs/gfs2/inode.c
>> >> @@ -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);
>> >
>> > 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.
>
> 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 <swhiteho@redhat.com>
> 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 <sachin.kamat@linaro.org>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Steven Whitehouse <swhiteho@redhat.com>
>
> diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
> index bbb2715..a01b8fd 100644
> --- a/fs/gfs2/inode.c
> +++ b/fs/gfs2/inode.c
> @@ -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;
> -- 
> 1.7.4

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

Cheers,
Rusty.