Received-SPF: pass (google.com: domain of linux-kernel+bounces-151046-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <b18c2e4b-bf61-41ad-b5f9-8e52d7651cca@web.de>
Date: Fri, 19 Apr 2024 08:15:25 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Wander Lairson Costa <wander@redhat.com>, kunit-dev@googlegroups.com,
 linux-kselftest@vger.kernel.org, kernel-janitors@vger.kernel.org,
 Brendan Higgins <brendan.higgins@linux.dev>, David Gow
 <davidgow@google.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
 Matti Vaittinen <mazziesaccount@gmail.com>,
 Maxime Ripard <mripard@kernel.org>, Rae Moar <rmoar@google.com>,
 Shuah Khan <skhan@linuxfoundation.org>
Cc: LKML <linux-kernel@vger.kernel.org>
References: <20240418210236.194190-3-wander@redhat.com>
Subject: Re: [PATCH v3 2/2] kunit: avoid memory leak on device register error
Content-Language: en-GB
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <20240418210236.194190-3-wander@redhat.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:3TBxCb/ezkM=;2p/RewvegXEDpauwRjt8uiNch3s
 BH9X8Z6Ifc02VySwy7N/3ezTh1/yaBHXewJLwqZ3XkTONVbAtZm53zjAUKPxXk+hAlQnHkuPl
 bAa9qPs72S/lN/xINB5gpjYiDZKSuW40KY4zWPjWms1ab69T7wMJ1+d+3TGxFXBNviWzX0KHI
 ibPH+jyyroU9jgHpNL4nPiTRiNjH1o70oXhUab9dYtdxxQbJL4ehHHdBEYsv0LDKoZ+1b/WWe
 YTCzyCe30fMQeelSK4iwj7m0xgVAcWCHahDE57LH3bKIo+rotVhH1ks99nSyPpxqHcmV0OnF1
 YwtsXYy9IzLC0fJL4vSyaHiq8xo6WDkERga++NBY60Tny654I1c+DPbtHSwZ8lvstFrqlk8AR
 DrXqpO9sTtlZ61PVHmGH7dsHt81+dBeM64diTS+fHJ/Ch5Gxo2WlVgx7gl1r3nfMCXjD+iW/n
 mfQUyfvlB+T0/EfzNLYCR02OHVLTEyGGjn/1PIHj8EfDMOzXFL542fj3xbP42djJxOizxRLlc
 /Ck03GxTgyIGoLiAZI1b1vsTT1MMaoHFzk1EXtf8nNk5HvNq/kROZ1gBg/d5jjuL1MYwLXVWf
 2mgccO115L2ARCk3npieuIVAzTMiOOEpZ+eHsd+Ed9FclhQ7EIQed92fAMtBCABOWiE0XUxNa
 JIwwTOnk3rJCzzwsq0DvYiD1To9omy6HAnfeROv+hXSejQvhuexmTGr8bvu4rMZkXo007Jtv/
 nVvO5I1t+ntMT6XEg4Y0uKIfM7QSWofLkg1SGiUSq+QjnvTP5CVLMR8qR4swN7gHW8/YrjqbT
 Rp3tRDZZ9YjYiLcDmU+zjdxNOP6+/wnPodaCdCYHuj0OQ=

> If the device register fails, free the allocated memory before
> returning.

Can a description variant (like the following) be more appropriate?

   Free the allocated memory (after a device registration failure)
   before returning.
   Thus add a jump target so that a bit of exception handling can be bette=
r
   reused at the end of this function implementation.


Would you like to replace the word =E2=80=9Cregister=E2=80=9D by =E2=80=9C=
registration=E2=80=9D also
in the summary phrase?


=E2=80=A6
> +++ b/lib/kunit/device.c
=E2=80=A6
> @@ -140,6 +138,9 @@ static struct kunit_device *kunit_device_register_in=
ternal(struct kunit *test,
>  	kunit_add_action(test, device_unregister_wrapper, &kunit_dev->dev);
>
>  	return kunit_dev;
> +error:
> +	kfree(kunit_dev);
> +	return ERR_PTR(err);
>  }
=E2=80=A6

I find it nicer to use a label like free_device.

Regards,
Markus