Re: [PATCH 8/19]: SCST SYSFS interface implementation

[Vladislav Bolkhovitin]

Greg KH, on 10/10/2010 01:20 AM wrote:
> On Sat, Oct 02, 2010 at 01:46:21AM +0400, Vladislav Bolkhovitin wrote:
>> This patch contains SYSFS interface implementation.
> 
> Nice, but you forgot to document it.  All sysfs changes need to be
> documented in Documentation/ABI/
> 
> Please add a file to this patch that does so.

I'll do. I didn't know about this. Thanks for pointing on it.

>> +static void scst_tgtt_release(struct kobject *kobj)
>> +{
>> +	struct scst_tgt_template *tgtt;
>> +
>> +	tgtt = container_of(kobj, struct scst_tgt_template, tgtt_kobj);
>> +	complete_all(&tgtt->tgtt_kobj_release_cmpl);
>> +	return;
> 
> Don't you also need to free the memory of your kobject here?
> 
>> +static void scst_tgt_release(struct kobject *kobj)
>> +{
>> +	struct scst_tgt *tgt;
>> +
>> +	tgt = container_of(kobj, struct scst_tgt, tgt_kobj);
>> +	complete_all(&tgt->tgt_kobj_release_cmpl);
>> +	return;
> 
> Same here, no kfree?
> 
>> +static void scst_acg_release(struct kobject *kobj)
>> +{
>> +	struct scst_acg *acg;
>> +
>> +	acg = container_of(kobj, struct scst_acg, acg_kobj);
>> +	complete_all(&acg->acg_kobj_release_cmpl);
> 
> And here.

Thanks for the review. In all those functions kobjects for simplicity
are embedded into the outer objects, so they will be freed as part of
the outer objects free. Hence, kfree() for the kobjects in the release
functions are not needed.

Thanks again,
Vlad
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/