diff mbox series

[5/6] drm/i915/sdvo: Reduce the size of the on stack buffers

Message ID 20190619180312.31817-5-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/6] drm/i915/sdvo: Fix handling if zero hbuf size | expand

Commit Message

Ville Syrjälä June 19, 2019, 6:03 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The strings we want to print to the on stack buffers should
be no more than
8 * 3 + strlen("(GET_SCALED_HDTV_RESOLUTION_SUPPORT)") + 1 = 61
bytes. So let's shrink the buffers down to 64 bytes.

Also switch the BUG_ON()s to WARN_ON()s if I made a mistake in
my arithmentic.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_sdvo.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

Comments

Chris Wilson June 19, 2019, 6:21 p.m. UTC | #1 
Quoting Ville Syrjala (2019-06-19 19:03:11)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The strings we want to print to the on stack buffers should
> be no more than
> 8 * 3 + strlen("(GET_SCALED_HDTV_RESOLUTION_SUPPORT)") + 1 = 61
> bytes. So let's shrink the buffers down to 64 bytes.
> 

max args_len does seem to 8, and it gets padded out to 8.

> Also switch the BUG_ON()s to WARN_ON()s if I made a mistake in
> my arithmentic.

With the command name macro, could it not do some thing like
BUILD_BUG_ON(sizeof(#cmd) > DBG_LEN) ?
-Chris
Ville Syrjälä June 24, 2019, 4:40 p.m. UTC | #2 
On Wed, Jun 19, 2019 at 07:21:48PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-06-19 19:03:11)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The strings we want to print to the on stack buffers should
> > be no more than
> > 8 * 3 + strlen("(GET_SCALED_HDTV_RESOLUTION_SUPPORT)") + 1 = 61
> > bytes. So let's shrink the buffers down to 64 bytes.
> > 
> 
> max args_len does seem to 8, and it gets padded out to 8.
> 
> > Also switch the BUG_ON()s to WARN_ON()s if I made a mistake in
> > my arithmentic.
> 
> With the command name macro, could it not do some thing like
> BUILD_BUG_ON(sizeof(#cmd) > DBG_LEN) ?

I couldn't think of a way to do that with the current struct
initialization, but we could borrow the I915_PARAMS_FOR_EACH()
trick here. Not sure it's worth the hassle though.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
index d1fd2bc01d82..df3582bab076 100644
--- a/drivers/gpu/drm/i915/display/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
@@ -401,12 +401,10 @@  static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
 				   const void *args, int args_len)
 {
 	int i, pos = 0;
-#define BUF_LEN 256
-	char buffer[BUF_LEN];
+	char buffer[64];
 
 #define BUF_PRINT(args...) \
-	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)
-
+	pos += snprintf(buffer + pos, max_t(int, sizeof(buffer) - pos, 0), args)
 
 	for (i = 0; i < args_len; i++) {
 		BUF_PRINT("%02X ", ((u8 *)args)[i]);
@@ -423,9 +421,9 @@  static void intel_sdvo_debug_write(struct intel_sdvo *intel_sdvo, u8 cmd,
 	if (i == ARRAY_SIZE(sdvo_cmd_names)) {
 		BUF_PRINT("(%02X)", cmd);
 	}
-	BUG_ON(pos >= BUF_LEN - 1);
+
+	WARN_ON(pos >= sizeof(buffer) - 1);
 #undef BUF_PRINT
-#undef BUF_LEN
 
 	DRM_DEBUG_KMS("%s: W: %02X %s\n", SDVO_NAME(intel_sdvo), cmd, buffer);
 }
@@ -521,8 +519,7 @@  static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 	u8 retry = 15; /* 5 quick checks, followed by 10 long checks */
 	u8 status;
 	int i, pos = 0;
-#define BUF_LEN 256
-	char buffer[BUF_LEN];
+	char buffer[64];
 
 	buffer[0] = '\0';
 
@@ -562,7 +559,7 @@  static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 	}
 
 #define BUF_PRINT(args...) \
-	pos += snprintf(buffer + pos, max_t(int, BUF_LEN - pos, 0), args)
+	pos += snprintf(buffer + pos, max_t(int, sizeof(buffer) - pos, 0), args)
 
 	if (status < ARRAY_SIZE(cmd_status_names))
 		BUF_PRINT("(%s)", cmd_status_names[status]);
@@ -580,9 +577,9 @@  static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 			goto log_fail;
 		BUF_PRINT(" %02X", ((u8 *)response)[i]);
 	}
-	BUG_ON(pos >= BUF_LEN - 1);
+
+	WARN_ON(pos >= sizeof(buffer) - 1);
 #undef BUF_PRINT
-#undef BUF_LEN
 
 	DRM_DEBUG_KMS("%s: R: %s\n", SDVO_NAME(intel_sdvo), buffer);
 	return true;