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;
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
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]; }
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
dri-devel@lists.freedesktop.org
-
F.A.Sulaiman -
Sam Ravnborg -
Simon Ser