Re: [PATCH v3 3/5] w1: ds2433: introduce a configuration structure

[Krzysztof Kozlowski]

On 30/11/2023 14:52, marc.ferland@xxxxxxxxx wrote:
>  static int w1_f23_add_slave(struct w1_slave *sl)
>  {
> -#ifdef CONFIG_W1_SLAVE_DS2433_CRC
>  	struct w1_f23_data *data;
>  
>  	data = kzalloc(sizeof(struct w1_f23_data), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
> +
> +	data->cfg = &config_f23;
> +
> +#ifdef CONFIG_W1_SLAVE_DS2433_CRC
> +	data->memory = kzalloc(data->cfg->eeprom_size, GFP_KERNEL);
> +	if (!data->memory) {
> +		kfree(data);
> +		return -ENOMEM;
> +	}
> +#endif /* CONFIG_W1_SLAVE_DS2433_CRC */
>  	sl->family_data = data;
>  
> -#endif	/* CONFIG_W1_SLAVE_DS2433_CRC */
>  	return 0;
>  }
>  
>  static void w1_f23_remove_slave(struct w1_slave *sl)
>  {
> +	struct w1_f23_data *data = sl->family_data;
>  #ifdef CONFIG_W1_SLAVE_DS2433_CRC
> -	kfree(sl->family_data);
> +	kfree(data->memory);
> +#endif /* CONFIG_W1_SLAVE_DS2433_CRC */
> +	kfree(data);
>  	sl->family_data = NULL;

This should be before kfree(), to match w1_f23_add_slave(). You can move
it now, since you store the pointer in data.

> -#endif	/* CONFIG_W1_SLAVE_DS2433_CRC */
>  }
>  
>  static const struct w1_family_ops w1_f23_fops = {

Best regards,
Krzysztof