1. Patchew
  2. linux
  3. View series

[PATCH] drm/vkms: Assert if vkms_config_create_*() fails

José Expósito posted 1 patch 1 month, 3 weeks ago 
drivers/gpu/drm/vkms/tests/vkms_config_test.c | 51 +++++++++++++++++--
1 file changed, 46 insertions(+), 5 deletions(-)
[PATCH] drm/vkms: Assert if vkms_config_create_*() fails
Posted by José Expósito 1 month, 3 weeks ago 
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>
---
 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)
-- 
2.50.1
Re: [PATCH] drm/vkms: Assert if vkms_config_create_*() fails
Posted by Louis Chauvet 4 weeks, 1 day ago 
On Mon, 11 Aug 2025 12:15:17 +0200, José Expósito wrote:
> Check that the value returned by the vkms_config_create_*() functions is
> valid. Otherwise, assert and finish the KUnit test.
> 
> 

Applied, thanks!

[1/1] drm/vkms: Assert if vkms_config_create_*() fails
      commit: 0d2902dfa31535c08d4976995d5b6890a8f43399

Best regards,
-- 
Louis Chauvet <louis.chauvet@bootlin.com>
Re: [PATCH] drm/vkms: Assert if vkms_config_create_*() fails
Posted by Louis Chauvet 1 month, 3 weeks ago 

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
Re: [PATCH] drm/vkms: Assert if vkms_config_create_*() fails
Posted by Dan Carpenter 1 month, 3 weeks ago 
On Mon, Aug 11, 2025 at 07:21:18PM +0200, Louis Chauvet wrote:
> 
> 
> 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.
> 

Looks good.  Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

> Side question: should we use __must_check in this case to warn at compile
> time?

We wouldn't normally...  It turns out the existing code was already
checking for errors so this is really just a "silence the checker"
patch.

regards,
dan carpenter
Re: [PATCH] drm/vkms: Assert if vkms_config_create_*() fails
Posted by José Expósito 1 month, 3 weeks ago 
Hi everyone,

On Tue, Aug 12, 2025 at 11:39:53AM +0300, Dan Carpenter wrote:
> On Mon, Aug 11, 2025 at 07:21:18PM +0200, Louis Chauvet wrote:
> > 
> > 
> > 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.
> > 
> 
> Looks good.  Thanks!
> 
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

Thanks to the both of you for your reviews :D

As a side note, I also couldn't make Smatch to report these warnings,
so thanks a lot for verifying that they are fixed.

Jose

> > Side question: should we use __must_check in this case to warn at compile
> > time?
> 
> We wouldn't normally...  It turns out the existing code was already
> checking for errors so this is really just a "silence the checker"
> patch.
> 
> regards,
> dan carpenter
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.