Re: [lustre-devel] [patch] staging: lustre: checking for NULL instead of IS_ERR

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2016/03/18, 13:28, "lustre-devel on behalf of Dilger, Andreas"
<lustre-devel-bounces@xxxxxxxxxxxxxxxx on behalf of
andreas.dilger@xxxxxxxxx> wrote:

>On 2016/03/17, 23:42, "Dan Carpenter" <dan.carpenter@xxxxxxxxxx> 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@xxxxxxxxxx>
>>
>>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@xxxxxxxxx>
>
>
>Cheers, Andreas
>-- 
>Andreas Dilger
>
>Lustre Principal Architect
>Intel High Performance Data Division
>
>
>_______________________________________________
>lustre-devel mailing list
>lustre-devel@xxxxxxxxxxxxxxxx
>http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
>


Cheers, Andreas
-- 
Andreas Dilger

Lustre Principal Architect
Intel High Performance Data Division


_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux