diff mbox

[04/12] video/hmdi: Clear the whole incoming buffer, not just the infoframe size

Message ID 1375817544-14565-5-git-send-email-damien.lespiau@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lespiau, Damien Aug. 6, 2013, 7:32 p.m. UTC
If the user if this API is providing a bigger buffer than the infoframe
size, it could be for a could reason. For instance it could be because
it gives the buffer that will be written to the hardware, up to the
maximum of an infoframe size.

Instead of just zeroing up to the infoframe size, let's zero the whole
incoming buffer as those extra bytes are also used to compute the
ECC and need to be 0.

Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
 drivers/video/hdmi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Ville Syrjälä Aug. 7, 2013, 10:56 a.m. UTC | #1
On Tue, Aug 06, 2013 at 08:32:16PM +0100, Damien Lespiau wrote:
> If the user if this API is providing a bigger buffer than the infoframe
> size, it could be for a could reason. For instance it could be because
> it gives the buffer that will be written to the hardware, up to the
> maximum of an infoframe size.
> 
> Instead of just zeroing up to the infoframe size, let's zero the whole
> incoming buffer as those extra bytes are also used to compute the
> ECC and need to be 0.
> 
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>

One concern that came to mind was someone needing to preserve the buffer
contents beyond the infoframe. But I guess if someone really needs to
do that, they can go and figure out the exact length of the infoframe
and pass that.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/video/hdmi.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index f7a85e5..635d569 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -84,7 +84,7 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer,
>  	if (size < length)
>  		return -ENOSPC;
>  
> -	memset(buffer, 0, length);
> +	memset(buffer, 0, size);
>  
>  	ptr[0] = frame->type;
>  	ptr[1] = frame->version;
> @@ -186,7 +186,7 @@ ssize_t hdmi_spd_infoframe_pack(struct hdmi_spd_infoframe *frame, void *buffer,
>  	if (size < length)
>  		return -ENOSPC;
>  
> -	memset(buffer, 0, length);
> +	memset(buffer, 0, size);
>  
>  	ptr[0] = frame->type;
>  	ptr[1] = frame->version;
> @@ -251,7 +251,7 @@ ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
>  	if (size < length)
>  		return -ENOSPC;
>  
> -	memset(buffer, 0, length);
> +	memset(buffer, 0, size);
>  
>  	if (frame->channels >= 2)
>  		channels = frame->channels - 1;
> @@ -308,7 +308,7 @@ ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
>  	if (size < length)
>  		return -ENOSPC;
>  
> -	memset(buffer, 0, length);
> +	memset(buffer, 0, size);
>  
>  	ptr[0] = frame->type;
>  	ptr[1] = frame->version;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lespiau, Damien Aug. 7, 2013, 11:02 a.m. UTC | #2
On Wed, Aug 07, 2013 at 01:56:58PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 06, 2013 at 08:32:16PM +0100, Damien Lespiau wrote:
> > If the user if this API is providing a bigger buffer than the infoframe
> > size, it could be for a could reason. For instance it could be because
> > it gives the buffer that will be written to the hardware, up to the
> > maximum of an infoframe size.
> > 
> > Instead of just zeroing up to the infoframe size, let's zero the whole
> > incoming buffer as those extra bytes are also used to compute the
> > ECC and need to be 0.
> > 
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> One concern that came to mind was someone needing to preserve the buffer
> contents beyond the infoframe. But I guess if someone really needs to
> do that, they can go and figure out the exact length of the infoframe
> and pass that.

Right, that was my thinking as well. We even have a macro for that now.
diff mbox

Patch

diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
index f7a85e5..635d569 100644
--- a/drivers/video/hdmi.c
+++ b/drivers/video/hdmi.c
@@ -84,7 +84,7 @@  ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer,
 	if (size < length)
 		return -ENOSPC;
 
-	memset(buffer, 0, length);
+	memset(buffer, 0, size);
 
 	ptr[0] = frame->type;
 	ptr[1] = frame->version;
@@ -186,7 +186,7 @@  ssize_t hdmi_spd_infoframe_pack(struct hdmi_spd_infoframe *frame, void *buffer,
 	if (size < length)
 		return -ENOSPC;
 
-	memset(buffer, 0, length);
+	memset(buffer, 0, size);
 
 	ptr[0] = frame->type;
 	ptr[1] = frame->version;
@@ -251,7 +251,7 @@  ssize_t hdmi_audio_infoframe_pack(struct hdmi_audio_infoframe *frame,
 	if (size < length)
 		return -ENOSPC;
 
-	memset(buffer, 0, length);
+	memset(buffer, 0, size);
 
 	if (frame->channels >= 2)
 		channels = frame->channels - 1;
@@ -308,7 +308,7 @@  ssize_t hdmi_vendor_infoframe_pack(struct hdmi_vendor_infoframe *frame,
 	if (size < length)
 		return -ENOSPC;
 
-	memset(buffer, 0, length);
+	memset(buffer, 0, size);
 
 	ptr[0] = frame->type;
 	ptr[1] = frame->version;