From: Louis Chauvet <louis.chauvet@bootlin.com>
To: "José Expósito" <jose.exposito89@gmail.com>,
"Dan Carpenter" <dan.carpenter@linaro.org>
Cc: hamohammed.sa@gmail.com, simona@ffwll.ch, melissa.srw@gmail.com,
maarten.lankhorst@linux.intel.com, mripard@kernel.org,
tzimmermann@suse.de, airlied@gmail.com,
dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/vkms: Assert if vkms_config_create_*() fails
Date: Mon, 11 Aug 2025 19:21:18 +0200 [thread overview]
Message-ID: <91587a35-f884-46ad-9869-126d3efbfc61@bootlin.com> (raw)
In-Reply-To: <20250811101529.150716-1-jose.exposito89@gmail.com>
Le 11/08/2025 à 12:15, José Expósito a écrit :
> Check that the value returned by the vkms_config_create_*() functions is
> valid. Otherwise, assert and finish the KUnit test.
>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/dri-devel/aJTL6IFEBaI8gqtH@stanley.mountain/
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
I am not sure on how to use smach, I don't have any warning at all for
the whole kernel, so I will wait for Dan Carpenter review before applying.
Side question: should we use __must_check in this case to warn at
compile time?
> ---
> drivers/gpu/drm/vkms/tests/vkms_config_test.c | 51 +++++++++++++++++--
> 1 file changed, 46 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_config_test.c b/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> index ff4566cf9925..b0d78a81d2df 100644
> --- a/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> +++ b/drivers/gpu/drm/vkms/tests/vkms_config_test.c
> @@ -200,6 +200,7 @@ static void vkms_config_test_get_planes(struct kunit *test)
> KUNIT_ASSERT_EQ(test, n_planes, 0);
>
> plane_cfg1 = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg1);
> vkms_config_for_each_plane(config, plane_cfg) {
> n_planes++;
> if (plane_cfg != plane_cfg1)
> @@ -209,6 +210,7 @@ static void vkms_config_test_get_planes(struct kunit *test)
> n_planes = 0;
>
> plane_cfg2 = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg2);
> vkms_config_for_each_plane(config, plane_cfg) {
> n_planes++;
> if (plane_cfg != plane_cfg1 && plane_cfg != plane_cfg2)
> @@ -242,6 +244,7 @@ static void vkms_config_test_get_crtcs(struct kunit *test)
> KUNIT_FAIL(test, "Unexpected CRTC");
>
> crtc_cfg1 = vkms_config_create_crtc(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg1);
> KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 1);
> vkms_config_for_each_crtc(config, crtc_cfg) {
> if (crtc_cfg != crtc_cfg1)
> @@ -249,6 +252,7 @@ static void vkms_config_test_get_crtcs(struct kunit *test)
> }
>
> crtc_cfg2 = vkms_config_create_crtc(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg2);
> KUNIT_ASSERT_EQ(test, vkms_config_get_num_crtcs(config), 2);
> vkms_config_for_each_crtc(config, crtc_cfg) {
> if (crtc_cfg != crtc_cfg1 && crtc_cfg != crtc_cfg2)
> @@ -280,6 +284,7 @@ static void vkms_config_test_get_encoders(struct kunit *test)
> KUNIT_ASSERT_EQ(test, n_encoders, 0);
>
> encoder_cfg1 = vkms_config_create_encoder(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder_cfg1);
> vkms_config_for_each_encoder(config, encoder_cfg) {
> n_encoders++;
> if (encoder_cfg != encoder_cfg1)
> @@ -289,6 +294,7 @@ static void vkms_config_test_get_encoders(struct kunit *test)
> n_encoders = 0;
>
> encoder_cfg2 = vkms_config_create_encoder(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder_cfg2);
> vkms_config_for_each_encoder(config, encoder_cfg) {
> n_encoders++;
> if (encoder_cfg != encoder_cfg1 && encoder_cfg != encoder_cfg2)
> @@ -324,6 +330,7 @@ static void vkms_config_test_get_connectors(struct kunit *test)
> KUNIT_ASSERT_EQ(test, n_connectors, 0);
>
> connector_cfg1 = vkms_config_create_connector(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, connector_cfg1);
> vkms_config_for_each_connector(config, connector_cfg) {
> n_connectors++;
> if (connector_cfg != connector_cfg1)
> @@ -333,6 +340,7 @@ static void vkms_config_test_get_connectors(struct kunit *test)
> n_connectors = 0;
>
> connector_cfg2 = vkms_config_create_connector(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, connector_cfg2);
> vkms_config_for_each_connector(config, connector_cfg) {
> n_connectors++;
> if (connector_cfg != connector_cfg1 &&
> @@ -370,7 +378,7 @@ static void vkms_config_test_invalid_plane_number(struct kunit *test)
>
> /* Invalid: Too many planes */
> for (n = 0; n <= 32; n++)
> - vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vkms_config_create_plane(config));
>
> KUNIT_EXPECT_FALSE(test, vkms_config_is_valid(config));
>
> @@ -395,6 +403,7 @@ static void vkms_config_test_valid_plane_type(struct kunit *test)
>
> /* Invalid: No primary plane */
> plane_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg);
> vkms_config_plane_set_type(plane_cfg, DRM_PLANE_TYPE_OVERLAY);
> err = vkms_config_plane_attach_crtc(plane_cfg, crtc_cfg);
> KUNIT_EXPECT_EQ(test, err, 0);
> @@ -402,11 +411,13 @@ static void vkms_config_test_valid_plane_type(struct kunit *test)
>
> /* Invalid: Multiple primary planes */
> plane_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg);
> vkms_config_plane_set_type(plane_cfg, DRM_PLANE_TYPE_PRIMARY);
> err = vkms_config_plane_attach_crtc(plane_cfg, crtc_cfg);
> KUNIT_EXPECT_EQ(test, err, 0);
>
> plane_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg);
> vkms_config_plane_set_type(plane_cfg, DRM_PLANE_TYPE_PRIMARY);
> err = vkms_config_plane_attach_crtc(plane_cfg, crtc_cfg);
> KUNIT_EXPECT_EQ(test, err, 0);
> @@ -419,11 +430,13 @@ static void vkms_config_test_valid_plane_type(struct kunit *test)
>
> /* Invalid: Multiple cursor planes */
> plane_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg);
> vkms_config_plane_set_type(plane_cfg, DRM_PLANE_TYPE_CURSOR);
> err = vkms_config_plane_attach_crtc(plane_cfg, crtc_cfg);
> KUNIT_EXPECT_EQ(test, err, 0);
>
> plane_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg);
> vkms_config_plane_set_type(plane_cfg, DRM_PLANE_TYPE_CURSOR);
> err = vkms_config_plane_attach_crtc(plane_cfg, crtc_cfg);
> KUNIT_EXPECT_EQ(test, err, 0);
> @@ -437,12 +450,16 @@ static void vkms_config_test_valid_plane_type(struct kunit *test)
> /* Invalid: Second CRTC without primary plane */
> crtc_cfg = vkms_config_create_crtc(config);
> encoder_cfg = vkms_config_create_encoder(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder_cfg);
> +
> err = vkms_config_encoder_attach_crtc(encoder_cfg, crtc_cfg);
> KUNIT_EXPECT_EQ(test, err, 0);
> KUNIT_EXPECT_FALSE(test, vkms_config_is_valid(config));
>
> /* Valid: Second CRTC with a primary plane */
> plane_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg);
> vkms_config_plane_set_type(plane_cfg, DRM_PLANE_TYPE_PRIMARY);
> err = vkms_config_plane_attach_crtc(plane_cfg, crtc_cfg);
> KUNIT_EXPECT_EQ(test, err, 0);
> @@ -486,7 +503,7 @@ static void vkms_config_test_invalid_crtc_number(struct kunit *test)
>
> /* Invalid: Too many CRTCs */
> for (n = 0; n <= 32; n++)
> - vkms_config_create_crtc(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vkms_config_create_crtc(config));
>
> KUNIT_EXPECT_FALSE(test, vkms_config_is_valid(config));
>
> @@ -509,7 +526,7 @@ static void vkms_config_test_invalid_encoder_number(struct kunit *test)
>
> /* Invalid: Too many encoders */
> for (n = 0; n <= 32; n++)
> - vkms_config_create_encoder(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vkms_config_create_encoder(config));
>
> KUNIT_EXPECT_FALSE(test, vkms_config_is_valid(config));
>
> @@ -531,12 +548,15 @@ static void vkms_config_test_valid_encoder_possible_crtcs(struct kunit *test)
>
> /* Invalid: Encoder without a possible CRTC */
> encoder_cfg = vkms_config_create_encoder(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder_cfg);
> KUNIT_EXPECT_FALSE(test, vkms_config_is_valid(config));
>
> /* Valid: Second CRTC with shared encoder */
> crtc_cfg2 = vkms_config_create_crtc(config);
> -
> plane_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg2);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg);
> +
> vkms_config_plane_set_type(plane_cfg, DRM_PLANE_TYPE_PRIMARY);
> err = vkms_config_plane_attach_crtc(plane_cfg, crtc_cfg2);
> KUNIT_EXPECT_EQ(test, err, 0);
> @@ -577,7 +597,7 @@ static void vkms_config_test_invalid_connector_number(struct kunit *test)
>
> /* Invalid: Too many connectors */
> for (n = 0; n <= 32; n++)
> - connector_cfg = vkms_config_create_connector(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, vkms_config_create_connector(config));
>
> KUNIT_EXPECT_FALSE(test, vkms_config_is_valid(config));
>
> @@ -669,13 +689,19 @@ static void vkms_config_test_plane_attach_crtc(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, config);
>
> overlay_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, overlay_cfg);
> vkms_config_plane_set_type(overlay_cfg, DRM_PLANE_TYPE_OVERLAY);
> +
> primary_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, primary_cfg);
> vkms_config_plane_set_type(primary_cfg, DRM_PLANE_TYPE_PRIMARY);
> +
> cursor_cfg = vkms_config_create_plane(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, cursor_cfg);
> vkms_config_plane_set_type(cursor_cfg, DRM_PLANE_TYPE_CURSOR);
>
> crtc_cfg = vkms_config_create_crtc(config);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg);
>
> /* No primary or cursor planes */
> KUNIT_EXPECT_NULL(test, vkms_config_crtc_primary_plane(config, crtc_cfg));
> @@ -735,6 +761,11 @@ static void vkms_config_test_plane_get_possible_crtcs(struct kunit *test)
> crtc_cfg1 = vkms_config_create_crtc(config);
> crtc_cfg2 = vkms_config_create_crtc(config);
>
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg1);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, plane_cfg2);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg1);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg2);
> +
> /* No possible CRTCs */
> vkms_config_plane_for_each_possible_crtc(plane_cfg1, idx, possible_crtc)
> KUNIT_FAIL(test, "Unexpected possible CRTC");
> @@ -799,6 +830,11 @@ static void vkms_config_test_encoder_get_possible_crtcs(struct kunit *test)
> crtc_cfg1 = vkms_config_create_crtc(config);
> crtc_cfg2 = vkms_config_create_crtc(config);
>
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder_cfg1);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder_cfg2);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg1);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, crtc_cfg2);
> +
> /* No possible CRTCs */
> vkms_config_encoder_for_each_possible_crtc(encoder_cfg1, idx, possible_crtc)
> KUNIT_FAIL(test, "Unexpected possible CRTC");
> @@ -863,6 +899,11 @@ static void vkms_config_test_connector_get_possible_encoders(struct kunit *test)
> encoder_cfg1 = vkms_config_create_encoder(config);
> encoder_cfg2 = vkms_config_create_encoder(config);
>
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, connector_cfg1);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, connector_cfg2);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder_cfg1);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, encoder_cfg2);
> +
> /* No possible encoders */
> vkms_config_connector_for_each_possible_encoder(connector_cfg1, idx,
> possible_encoder)
--
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
next prev parent reply other threads:[~2025-08-11 17:21 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-11 10:15 José Expósito
2025-08-11 17:21 ` Louis Chauvet [this message]
2025-08-12 8:39 ` Dan Carpenter
2025-08-14 10:48 ` José Expósito
2025-09-05 17:09 ` Louis Chauvet
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=91587a35-f884-46ad-9869-126d3efbfc61@bootlin.com \
--to=louis.chauvet@bootlin.com \
--cc=airlied@gmail.com \
--cc=dan.carpenter@linaro.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=hamohammed.sa@gmail.com \
--cc=jose.exposito89@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=melissa.srw@gmail.com \
--cc=mripard@kernel.org \
--cc=simona@ffwll.ch \
--cc=tzimmermann@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®