[PATCH] GPU:drm: returning ENOMEM

[PATCH] GPU:drm: returning ENOMEM

[F.A.Sulaiman]

When memory allocation is failed this patch returns out of memory error instead of -1.

Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>
---
 drivers/gpu/drm/udl/udl_connector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index 3750fd216131..afebab6186ab 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -24,7 +24,7 @@ static int udl_get_edid_block(void *data, u8 *buf, unsigned int block,
 
 	read_buff = kmalloc(2, GFP_KERNEL);
 	if (!read_buff)
-		return -1;
+		return -ENOMEM;
 
 	for (i = 0; i < len; i++) {
 		int bval = (i + block * EDID_LENGTH) << 8;
-- 
2.17.1

Re: [PATCH] GPU:drm: returning ENOMEM

[Sam Ravnborg]

Hi F.A.Sulaiman,
On Sat, Aug 28, 2021 at 09:10:27PM +0530, F.A.Sulaiman wrote:
> When memory allocation is failed this patch returns out of memory error instead of -1.
> 
> Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>
> ---
>  drivers/gpu/drm/udl/udl_connector.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
> index 3750fd216131..afebab6186ab 100644
> --- a/drivers/gpu/drm/udl/udl_connector.c
> +++ b/drivers/gpu/drm/udl/udl_connector.c
> @@ -24,7 +24,7 @@ static int udl_get_edid_block(void *data, u8 *buf, unsigned int block,
>  
>  	read_buff = kmalloc(2, GFP_KERNEL);
>  	if (!read_buff)
> -		return -1;
> +		return -ENOMEM;
>  
>  	for (i = 0; i < len; i++) {
>  		int bval = (i + block * EDID_LENGTH) << 8;

The function is only used in a context where the return value is checked
if it is o or not zero.
But it seems prudent to return a proper error code anyways.

Please revisit the function and fix the other return -1;
Propagate the error from the called function and avoid the hardcoded -1.

	Sam

[PATCH v2] GPU:DRM: returning ENOMEM

[F.A.Sulaiman]

check the return value and pass the proper error code.

Signed-off-by: F.A. SULAIMAN <asha.16@itfac.mrt.ac.lk>
---
 drivers/gpu/drm/udl/udl_connector.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index cdc1c42e1669..857d2c97ef0e 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -23,7 +23,7 @@ static int udl_get_edid_block(void *data, u8 *buf, unsigned int block,
 
 	read_buff = kmalloc(2, GFP_KERNEL);
 	if (!read_buff)
-		return -1;
+		return -ENOMEM;
 
 	for (i = 0; i < len; i++) {
 		int bval = (i + block * EDID_LENGTH) << 8;
@@ -31,11 +31,16 @@ static int udl_get_edid_block(void *data, u8 *buf, unsigned int block,
 				      usb_rcvctrlpipe(udl->udev, 0),
 					  (0x02), (0x80 | (0x02 << 5)), bval,
 					  0xA1, read_buff, 2, HZ);
-		if (ret < 1) {
+		if (ret == 0) {
+			DRM_ERROR("Reading EDID block %d returned empty result\n", i);
+			kfree(read_buff);
+			return -EINVAL;
+		} else if (ret < 0) {
 			DRM_ERROR("Read EDID byte %d failed err %x\n", i, ret);
 			kfree(read_buff);
-			return -1;
+			return ret;
 		}
+
 		buf[i] = read_buff[1];
 	}
 
-- 
2.17.1

Re: [PATCH v2] GPU:DRM: returning ENOMEM

[Simon Ser]

Maybe the commit message can be improved a bit? Add a prefix to make it
clear this is about the udl driver, make it clear this is about the
udl_get_edid_block function. The new `return ret` statement may return
something different from ENOMEM.

This would give something among these lines:

    drm/udl: return -errno in udl_get_edid_block