* [patch] staging: lustre: checking for NULL instead of IS_ERR
@ 2016-03-18 5:42 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2016-03-18 5:42 UTC (permalink / raw)
To: lustre-devel
lustre_cfg_new() returns error pointers on error, it never returns NULL.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 65caffe..b7dc872 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1282,7 +1282,7 @@ static int mgc_apply_recover_logs(struct obd_device *mgc,
rc = -ENOMEM;
lcfg = lustre_cfg_new(LCFG_PARAM, &bufs);
- if (!lcfg) {
+ if (IS_ERR(lcfg)) {
CERROR("mgc: cannot allocate memory\n");
break;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* [lustre-devel] [patch] staging: lustre: checking for NULL instead of IS_ERR
@ 2016-03-18 5:42 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2016-03-18 5:42 UTC (permalink / raw)
To: lustre-devel
lustre_cfg_new() returns error pointers on error, it never returns NULL.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index 65caffe..b7dc872 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -1282,7 +1282,7 @@ static int mgc_apply_recover_logs(struct obd_device *mgc,
rc = -ENOMEM;
lcfg = lustre_cfg_new(LCFG_PARAM, &bufs);
- if (!lcfg) {
+ if (IS_ERR(lcfg)) {
CERROR("mgc: cannot allocate memory\n");
break;
}
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [patch] staging: lustre: checking for NULL instead of IS_ERR
2016-03-18 5:42 ` [lustre-devel] " Dan Carpenter
@ 2016-03-18 19:28 ` Dilger, Andreas
-1 siblings, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2016-03-18 19:28 UTC (permalink / raw)
To: lustre-devel
On 2016/03/17, 23:42, "Dan Carpenter" <dan.carpenter@oracle.com> wrote:
>lustre_cfg_new() returns error pointers on error, it never returns NULL.
>
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
>diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>index 65caffe..b7dc872 100644
>--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>@@ -1282,7 +1282,7 @@ static int mgc_apply_recover_logs(struct obd_device
>*mgc,
>
> rc = -ENOMEM;
> lcfg = lustre_cfg_new(LCFG_PARAM, &bufs);
>- if (!lcfg) {
>+ if (IS_ERR(lcfg)) {
> CERROR("mgc: cannot allocate memory\n");
> break;
> }
Looks good. Thanks for the patch.
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel High Performance Data Division
^ permalink raw reply [flat|nested] 6+ messages in thread* [lustre-devel] [patch] staging: lustre: checking for NULL instead of IS_ERR
@ 2016-03-18 19:28 ` Dilger, Andreas
0 siblings, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2016-03-18 19:28 UTC (permalink / raw)
To: lustre-devel
On 2016/03/17, 23:42, "Dan Carpenter" <dan.carpenter@oracle.com> wrote:
>lustre_cfg_new() returns error pointers on error, it never returns NULL.
>
>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
>diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>index 65caffe..b7dc872 100644
>--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>@@ -1282,7 +1282,7 @@ static int mgc_apply_recover_logs(struct obd_device
>*mgc,
>
> rc = -ENOMEM;
> lcfg = lustre_cfg_new(LCFG_PARAM, &bufs);
>- if (!lcfg) {
>+ if (IS_ERR(lcfg)) {
> CERROR("mgc: cannot allocate memory\n");
> break;
> }
Looks good. Thanks for the patch.
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel High Performance Data Division
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [lustre-devel] [patch] staging: lustre: checking for NULL instead of IS_ERR
2016-03-18 19:28 ` [lustre-devel] " Dilger, Andreas
@ 2016-03-18 19:38 ` Dilger, Andreas
-1 siblings, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2016-03-18 19:38 UTC (permalink / raw)
To: lustre-devel
On 2016/03/18, 13:28, "lustre-devel on behalf of Dilger, Andreas"
<lustre-devel-bounces@lists.lustre.org on behalf of
andreas.dilger@intel.com> wrote:
>On 2016/03/17, 23:42, "Dan Carpenter" <dan.carpenter@oracle.com> wrote:
>
>>lustre_cfg_new() returns error pointers on error, it never returns NULL.
Hmm, looking at the broader context, I see there are other callers of
lustre_cfg_new() that are checking for NULL, and in some cases not
checking the return code at all.
I'm not sure why Peng Tao changed the return value of this function to
ERR_PTR() instead of NULL, but it may be better to have lustre_cfg_new()
return NULL on failure to avoid changing more code, and because it is
essentially an alloc+init function and the only error it could ever
return is -ENOMEM and typically allocator functions return NULL.
Cheers, Andreas
>>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>>diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>>b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>>index 65caffe..b7dc872 100644
>>--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>>+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>>@@ -1282,7 +1282,7 @@ static int mgc_apply_recover_logs(struct obd_device
>>*mgc,
>>
>> rc = -ENOMEM;
>> lcfg = lustre_cfg_new(LCFG_PARAM, &bufs);
>>- if (!lcfg) {
>>+ if (IS_ERR(lcfg)) {
>> CERROR("mgc: cannot allocate memory\n");
>> break;
>> }
>
>Looks good. Thanks for the patch.
>
>Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
>
>
>Cheers, Andreas
>--
>Andreas Dilger
>
>Lustre Principal Architect
>Intel High Performance Data Division
>
>
>_______________________________________________
>lustre-devel mailing list
>lustre-devel@lists.lustre.org
>http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
>
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel High Performance Data Division
^ permalink raw reply [flat|nested] 6+ messages in thread* [lustre-devel] [patch] staging: lustre: checking for NULL instead of IS_ERR
@ 2016-03-18 19:38 ` Dilger, Andreas
0 siblings, 0 replies; 6+ messages in thread
From: Dilger, Andreas @ 2016-03-18 19:38 UTC (permalink / raw)
To: lustre-devel
On 2016/03/18, 13:28, "lustre-devel on behalf of Dilger, Andreas"
<lustre-devel-bounces at lists.lustre.org on behalf of
andreas.dilger@intel.com> wrote:
>On 2016/03/17, 23:42, "Dan Carpenter" <dan.carpenter@oracle.com> wrote:
>
>>lustre_cfg_new() returns error pointers on error, it never returns NULL.
Hmm, looking at the broader context, I see there are other callers of
lustre_cfg_new() that are checking for NULL, and in some cases not
checking the return code at all.
I'm not sure why Peng Tao changed the return value of this function to
ERR_PTR() instead of NULL, but it may be better to have lustre_cfg_new()
return NULL on failure to avoid changing more code, and because it is
essentially an alloc+init function and the only error it could ever
return is -ENOMEM and typically allocator functions return NULL.
Cheers, Andreas
>>Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>>
>>diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>>b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>>index 65caffe..b7dc872 100644
>>--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
>>+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
>>@@ -1282,7 +1282,7 @@ static int mgc_apply_recover_logs(struct obd_device
>>*mgc,
>>
>> rc = -ENOMEM;
>> lcfg = lustre_cfg_new(LCFG_PARAM, &bufs);
>>- if (!lcfg) {
>>+ if (IS_ERR(lcfg)) {
>> CERROR("mgc: cannot allocate memory\n");
>> break;
>> }
>
>Looks good. Thanks for the patch.
>
>Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
>
>
>Cheers, Andreas
>--
>Andreas Dilger
>
>Lustre Principal Architect
>Intel High Performance Data Division
>
>
>_______________________________________________
>lustre-devel mailing list
>lustre-devel at lists.lustre.org
>http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
>
Cheers, Andreas
--
Andreas Dilger
Lustre Principal Architect
Intel High Performance Data Division
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-18 19:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-18 5:42 [patch] staging: lustre: checking for NULL instead of IS_ERR Dan Carpenter
2016-03-18 5:42 ` [lustre-devel] " Dan Carpenter
2016-03-18 19:28 ` Dilger, Andreas
2016-03-18 19:28 ` [lustre-devel] " Dilger, Andreas
2016-03-18 19:38 ` Dilger, Andreas
2016-03-18 19:38 ` Dilger, Andreas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.