Re: [PATCH 2/2] target: use kstrdup()

["Nicholas A. Bellinger" <nab@xxxxxxxxxxxxxxx>]

On Thu, 2012-11-29 at 20:29 +0100, Sebastian Andrzej Siewior wrote:
> The kmalloc() + strlen() + memcpy() block is what kstrdup() does as
> well.

<nod>

> While here I also removed the "to NULL assignment" of pointers which are
> fed to kfree or thrown away anyway.
> 

Comments below..

> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
>  drivers/target/iscsi/iscsi_target_parameters.c |   16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/target/iscsi/iscsi_target_parameters.c b/drivers/target/iscsi/iscsi_target_parameters.c
> index 9602b20..652a608 100644
> --- a/drivers/target/iscsi/iscsi_target_parameters.c
> +++ b/drivers/target/iscsi/iscsi_target_parameters.c
> @@ -153,22 +153,18 @@ static struct iscsi_param *iscsi_set_default_param(struct iscsi_param_list *para
>  		goto out;
>  	}
>  
> -	param->name = kzalloc(strlen(name) + 1, GFP_KERNEL);
> +	param->name = kstrdup(name, GFP_KERNEL);
>  	if (!param->name) {
>  		pr_err("Unable to allocate memory for parameter name.\n");
>  		goto out;
>  	}
>  
> -	param->value = kzalloc(strlen(value) + 1, GFP_KERNEL);
> +	param->value = kstrdup(value, GFP_KERNEL);
>  	if (!param->value) {
>  		pr_err("Unable to allocate memory for parameter value.\n");
>  		goto out;
>  	}
>  
> -	memcpy(param->name, name, strlen(name));
> -	param->name[strlen(name)] = '\0';
> -	memcpy(param->value, value, strlen(value));
> -	param->value[strlen(value)] = '\0';

Mmmm, I pretty sure that iSCSI still wants the '\0' terminator included
at the end param->[name, value].

So applying this patch, but keeping this around for now.

>  	param->phase		= phase;
>  	param->scope		= scope;
>  	param->sender		= sender;
> @@ -634,11 +630,8 @@ void iscsi_release_param_list(struct iscsi_param_list *param_list)
>  		list_del(&param->p_list);
>  
>  		kfree(param->name);
> -		param->name = NULL;
>  		kfree(param->value);
> -		param->value = NULL;
>  		kfree(param);
> -		param = NULL;
>  	}
>  
>  	iscsi_release_extra_responses(param_list);
> @@ -686,15 +679,12 @@ int iscsi_update_param_value(struct iscsi_param *param, char *value)
>  {
>  	kfree(param->value);
>  
> -	param->value = kzalloc(strlen(value) + 1, GFP_KERNEL);
> +	param->value = kstrdup(value, GFP_KERNEL);
>  	if (!param->value) {
>  		pr_err("Unable to allocate memory for value.\n");
>  		return -ENOMEM;
>  	}
>  
> -	memcpy(param->value, value, strlen(value));
> -	param->value[strlen(value)] = '\0';
> -

Ditto here as well.

>  	pr_debug("iSCSI Parameter updated to %s=%s\n",
>  			param->name, param->value);
>  	return 0;


--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html