Re: [lustre-devel] [PATCH 05/12] staging: lustre: Optimize error handling in class_register_type()

[Oleg Drokin]

On Jul 26, 2016, at 3:11 PM, Oleg Drokin wrote:

> 
> On Jul 26, 2016, at 3:05 PM, SF Markus Elfring wrote:
> 
>> From: Markus Elfring 
>> Date: Tue, 26 Jul 2016 14:23:23 +0200
>> 
>> Return a constant error code without storing it in the local variable "rc"
>> after a failed memory allocation at the beginning of this function.
>> 
>> Signed-off-by: Markus Elfring 
>> ---
>> drivers/staging/lustre/lustre/obdclass/genops.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c 
>> b/drivers/staging/lustre/lustre/obdclass/genops.c
>> index fd5e61f..4752091 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
>> @@ -152,10 +152,9 @@ int class_register_type(struct obd_ops *dt_ops, struct 
>> md_ops *md_ops,
>>  return -EEXIST;
>>  }
>> 
>> -rc = -ENOMEM;
> 
> NAK.
> when you do this, the next statement below breaks:

Ah, I see there was patch 4 before patch 5 that actually placed rc assignments
everywhere.

So I guess it is fine after all.

Sorry for the noise.

> 
>>  type = kzalloc(sizeof(*type), GFP_NOFS);
>>  if (!type)
>> -return rc;
>> +return -ENOMEM;
>> 
>>  type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
>>  if (!type->typ_dt_ops) {
> …
>goto failed;
> 
> failed:
> …
> return rc;
> 
> So we are now returning an unitialized rc, did you get a gcc warning about it 
> when compiling?
> ___
> lustre-devel mailing list
> lustre-de...@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

Re: [lustre-devel] [PATCH 05/12] staging: lustre: Optimize error handling in class_register_type()

[Oleg Drokin]

On Jul 26, 2016, at 3:11 PM, Oleg Drokin wrote:

> 
> On Jul 26, 2016, at 3:05 PM, SF Markus Elfring wrote:
> 
>> From: Markus Elfring 
>> Date: Tue, 26 Jul 2016 14:23:23 +0200
>> 
>> Return a constant error code without storing it in the local variable "rc"
>> after a failed memory allocation at the beginning of this function.
>> 
>> Signed-off-by: Markus Elfring 
>> ---
>> drivers/staging/lustre/lustre/obdclass/genops.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c 
>> b/drivers/staging/lustre/lustre/obdclass/genops.c
>> index fd5e61f..4752091 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
>> @@ -152,10 +152,9 @@ int class_register_type(struct obd_ops *dt_ops, struct 
>> md_ops *md_ops,
>>  return -EEXIST;
>>  }
>> 
>> -rc = -ENOMEM;
> 
> NAK.
> when you do this, the next statement below breaks:

Ah, I see there was patch 4 before patch 5 that actually placed rc assignments
everywhere.

So I guess it is fine after all.

Sorry for the noise.

> 
>>  type = kzalloc(sizeof(*type), GFP_NOFS);
>>  if (!type)
>> -return rc;
>> +return -ENOMEM;
>> 
>>  type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
>>  if (!type->typ_dt_ops) {
> …
>goto failed;
> 
> failed:
> …
> return rc;
> 
> So we are now returning an unitialized rc, did you get a gcc warning about it 
> when compiling?
> ___
> lustre-devel mailing list
> lustre-de...@lists.lustre.org
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org

[PATCH 05/12] staging: lustre: Optimize error handling in class_register_type()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Jul 2016 14:23:23 +0200

Return a constant error code without storing it in the local variable "rc"
after a failed memory allocation at the beginning of this function.

Signed-off-by: Markus Elfring 
---
 drivers/staging/lustre/lustre/obdclass/genops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c 
b/drivers/staging/lustre/lustre/obdclass/genops.c
index fd5e61f..4752091 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -152,10 +152,9 @@ int class_register_type(struct obd_ops *dt_ops, struct 
md_ops *md_ops,
return -EEXIST;
}
 
-   rc = -ENOMEM;
type = kzalloc(sizeof(*type), GFP_NOFS);
if (!type)
-   return rc;
+   return -ENOMEM;
 
type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
if (!type->typ_dt_ops) {
-- 
2.9.2

[PATCH 05/12] staging: lustre: Optimize error handling in class_register_type()

[SF Markus Elfring]

From: Markus Elfring 
Date: Tue, 26 Jul 2016 14:23:23 +0200

Return a constant error code without storing it in the local variable "rc"
after a failed memory allocation at the beginning of this function.

Signed-off-by: Markus Elfring 
---
 drivers/staging/lustre/lustre/obdclass/genops.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c 
b/drivers/staging/lustre/lustre/obdclass/genops.c
index fd5e61f..4752091 100644
--- a/drivers/staging/lustre/lustre/obdclass/genops.c
+++ b/drivers/staging/lustre/lustre/obdclass/genops.c
@@ -152,10 +152,9 @@ int class_register_type(struct obd_ops *dt_ops, struct 
md_ops *md_ops,
return -EEXIST;
}
 
-   rc = -ENOMEM;
type = kzalloc(sizeof(*type), GFP_NOFS);
if (!type)
-   return rc;
+   return -ENOMEM;
 
type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
if (!type->typ_dt_ops) {
-- 
2.9.2

Re: [PATCH 05/12] staging: lustre: Optimize error handling in class_register_type()

[Oleg Drokin]

On Jul 26, 2016, at 3:05 PM, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Tue, 26 Jul 2016 14:23:23 +0200
> 
> Return a constant error code without storing it in the local variable "rc"
> after a failed memory allocation at the beginning of this function.
> 
> Signed-off-by: Markus Elfring 
> ---
> drivers/staging/lustre/lustre/obdclass/genops.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c 
> b/drivers/staging/lustre/lustre/obdclass/genops.c
> index fd5e61f..4752091 100644
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -152,10 +152,9 @@ int class_register_type(struct obd_ops *dt_ops, struct 
> md_ops *md_ops,
>   return -EEXIST;
>   }
> 
> - rc = -ENOMEM;

NAK.
when you do this, the next statement below breaks:

>   type = kzalloc(sizeof(*type), GFP_NOFS);
>   if (!type)
> - return rc;
> + return -ENOMEM;
> 
>   type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
>   if (!type->typ_dt_ops) {
…
goto failed;

 failed:
…
return rc;

So we are now returning an unitialized rc, did you get a gcc warning about it 
when compiling?

Re: [PATCH 05/12] staging: lustre: Optimize error handling in class_register_type()

[Oleg Drokin]

On Jul 26, 2016, at 3:05 PM, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Tue, 26 Jul 2016 14:23:23 +0200
> 
> Return a constant error code without storing it in the local variable "rc"
> after a failed memory allocation at the beginning of this function.
> 
> Signed-off-by: Markus Elfring 
> ---
> drivers/staging/lustre/lustre/obdclass/genops.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdclass/genops.c 
> b/drivers/staging/lustre/lustre/obdclass/genops.c
> index fd5e61f..4752091 100644
> --- a/drivers/staging/lustre/lustre/obdclass/genops.c
> +++ b/drivers/staging/lustre/lustre/obdclass/genops.c
> @@ -152,10 +152,9 @@ int class_register_type(struct obd_ops *dt_ops, struct 
> md_ops *md_ops,
>   return -EEXIST;
>   }
> 
> - rc = -ENOMEM;

NAK.
when you do this, the next statement below breaks:

>   type = kzalloc(sizeof(*type), GFP_NOFS);
>   if (!type)
> - return rc;
> + return -ENOMEM;
> 
>   type->typ_dt_ops = kzalloc(sizeof(*type->typ_dt_ops), GFP_NOFS);
>   if (!type->typ_dt_ops) {
…
goto failed;

 failed:
…
return rc;

So we are now returning an unitialized rc, did you get a gcc warning about it 
when compiling?