Re: [PATCH 2/2] drivers/base: fix iterator cleanup in attribute_container_find_class_device (v2)

[Greg KH]

On Thu, Feb 27, 2025 at 06:11:29PM +0530, Ayaan Mirza Baig wrote:
> This patch fixes a long-standing FIXME by refactoring
> the function to ensure klist_iter_exit() is always
> invoked once via a unified cleanup path
> 
> The patch has been tested thoroughly with a minimal
> Debain System and scsi devices and GDB.
> 
> Tested-by: Ayaan Mirza Baig <ayaanmirzabaig85@xxxxxxxxx>
> Signed-off-by: Ayaan Mirza Baig <ayaanmirzabaig85@xxxxxxxxx>

No need to duplicate this here, if you sign-off it's implicit that you
tested it :)

> ---
>  drivers/base/attribute_container.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/base/attribute_container.c b/drivers/base/attribute_container.c
> index 69637b2ec3bc..fbf7fd45651e 100644
> --- a/drivers/base/attribute_container.c
> +++ b/drivers/base/attribute_container.c
> @@ -492,12 +492,11 @@ attribute_container_find_class_device(struct attribute_container *cont,
>  	klist_for_each_entry(ic, &cont->containers, node, &iter) {
>  		if (ic->classdev.parent == dev) {
>  			cdev = &ic->classdev;
> -			/* FIXME: must exit iterator then break */
> -			klist_iter_exit(&iter);
> -			break;
> +			goto out;
>  		}
>  	}
> -
> +out:
> +	klist_iter_exit(&iter);

Are you sure this actually does something different?  Why is the FIXME
there and why does this "simple" change solve the issue?  I don't
understand how this change resolves the problem.

In other words, you're going to have to document this a lot better in
the changelog message please.

thanks,

greg k-h