Re: [PATCH] drivers: base: Remove statistics group if encryption group not created

[Ewan Milne]

Hi Justin-

Thanks for looking at this.  I don't think I explained this well
enough.  See below.

On Mon, Jun 8, 2026 at 5:12 PM Justin Tee <justintee8345@xxxxxxxxx> wrote:
>
> Hi Ewan,
>
> Thanks for bringing this to attention.  So, I have two comments:
>
> 1.) In the err_del_statistics label, should we check for if
> (tcont->statistics) and only then sysfs_remove_group(&classdev->kobj,
> tcont->statistics) could be called?
>
> err_del_statistics:
>          if (tcont->statistics)
>                  sysfs_remove_group(&classdev->kobj, tcont->statistics);
>

I think the patch is correct as-is, because the only way to get to the
err_del_statistics: label
is from an explicit goto if an error is returned when attempting to
create the encryption group.
In order to get there the creation of the statistics group had to have
been successful first.

> 2.) In general, if the tcont->encryption sysfs group couldn’t be
> created, why would we remove the tcont->statistics sysfs group? The
> tcont->statistics and tcont->encryption are independent of each other.
> Just because the tcont->encryption sysfs group couldn’t be created
> shouldn’t mean that we need to remove the tcont->statistics sysfs group
> too.  And, clean up of the tcont->statistics sysfs group is not lost.
> When transport_remove_classdev is called, the tcont->statistics sysfs
> group is cleaned up at that time.

If the encryption group could not be created, transport_add_class_device()
would return an error, and the function would have called both
attribute_container_class_device_del() and the tclass->remove() functions.

The basic issue is that if there is an error creating the encryption group,
we clean everything up except we do not clean up the statistics group
that was created and is effectively orphaned.

Or so I think.  If the intention was to allow the creation of the encryption
group to be optional, then the code could be changed to not use a goto
to return an error.  But I am not sure what other effects this might have.

-Ewan

>
> Regards,
> Justin
>