Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Brahmajit]

While building the latest stable release with GCC 15 I'm getting an
build error with (I'm guessing) the drm module. This is probably due to
combination of GCC 15's introduction of a new warning
-Wunterminated-string-initialization, -Werror and -Wextra

In the drivers/gpu/drm/display/drm_dp_dual_mode_helper.c file there is a
code section on 163

static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
	"DP-HDMI ADAPTOR\x04";

Its from the is_hdmi_adaptor() function.
I read a little about C strings and it seems like we are assigning more
than 16 characters to the array dp_dual_mode_hdmi_id, and
DP_DUAL_MODE_HDMI_ID_LEN is defined as 16.

I'm guessing this has something to do with how termination happens in
char array vs char* and/or null termination

-- 
Regards,
Brahmajit

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Brahmajit]

On 01.10.2024 11:45, Brahmajit wrote:
> While building the latest stable release with GCC 15 I'm getting an
> build error with (I'm guessing) the drm module. This is probably due to
> combination of GCC 15's introduction of a new warning
> -Wunterminated-string-initialization, -Werror and -Wextra
> 
> In the drivers/gpu/drm/display/drm_dp_dual_mode_helper.c file there is a
> code section on 163
> 
> static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> 	"DP-HDMI ADAPTOR\x04";
> 
> Its from the is_hdmi_adaptor() function.
> I read a little about C strings and it seems like we are assigning more
> than 16 characters to the array dp_dual_mode_hdmi_id, and
> DP_DUAL_MODE_HDMI_ID_LEN is defined as 16.
> 
> I'm guessing this has something to do with how termination happens in
> char array vs char* and/or null termination

Some references I found:
- https://lore.kernel.org/linux-btrfs/5mnphkdvheudccjtiatrbjbkqtw54s2wkpeqevj3rqthdqlwyw@sjvn4wn52qki/
- https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115185

-- 
Regards,
Brahmajit

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Ville Syrjälä]

On Tue, Oct 01, 2024 at 11:48:56AM +0530, Brahmajit wrote:
> On 01.10.2024 11:45, Brahmajit wrote:
> > While building the latest stable release with GCC 15 I'm getting an
> > build error with (I'm guessing) the drm module. This is probably due to
> > combination of GCC 15's introduction of a new warning
> > -Wunterminated-string-initialization, -Werror and -Wextra
> > 
> > In the drivers/gpu/drm/display/drm_dp_dual_mode_helper.c file there is a
> > code section on 163
> > 
> > static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> > 	"DP-HDMI ADAPTOR\x04";
> > 
> > Its from the is_hdmi_adaptor() function.
> > I read a little about C strings and it seems like we are assigning more
> > than 16 characters to the array dp_dual_mode_hdmi_id, and
> > DP_DUAL_MODE_HDMI_ID_LEN is defined as 16.
> > 
> > I'm guessing this has something to do with how termination happens in
> > char array vs char* and/or null termination
> 
> Some references I found:
> - https://lore.kernel.org/linux-btrfs/5mnphkdvheudccjtiatrbjbkqtw54s2wkpeqevj3rqthdqlwyw@sjvn4wn52qki/
> - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115185

Looks like gcc is warning about perfectly legal code
(as far as the C standard goes).

But we could work around by adding room for the '\0' and
s/sizeof(...)/DP_DUAL_MODE_HDMI_ID_LEN/ in the memcmp().

-- 
Ville Syrjälä
Intel

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Brahmajit]

On 01.10.2024 16:28, Ville Syrjälä wrote:
> Looks like gcc is warning about perfectly legal code
> (as far as the C standard goes).
> 
> But we could work around by adding room for the '\0' and
> s/sizeof(...)/DP_DUAL_MODE_HDMI_ID_LEN/ in the memcmp().

Sorry I'm new, but can we do something like this?

--- a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
@@ -158,13 +158,13 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
 }
 EXPORT_SYMBOL(drm_dp_dual_mode_write);
 
-static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
+static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1])
 {
-	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
+	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1] =
 		"DP-HDMI ADAPTOR\x04";
 
 	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
-		      sizeof(dp_dual_mode_hdmi_id)) == 0;
+		      DP_DUAL_MODE_HDMI_ID_LEN+1) == 0;
 }
 
 static bool is_type1_adaptor(uint8_t adaptor_id)

-- 
Regards,
Brahmajit

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Ville Syrjälä]

On Tue, Oct 01, 2024 at 07:18:23PM +0530, Brahmajit wrote:
> On 01.10.2024 16:28, Ville Syrjälä wrote:
> > Looks like gcc is warning about perfectly legal code
> > (as far as the C standard goes).
> > 
> > But we could work around by adding room for the '\0' and
> > s/sizeof(...)/DP_DUAL_MODE_HDMI_ID_LEN/ in the memcmp().
> 
> Sorry I'm new, but can we do something like this?
> 
> --- a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
> @@ -158,13 +158,13 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
>  }
>  EXPORT_SYMBOL(drm_dp_dual_mode_write);
>  
> -static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> +static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1])

This should not have the +1

>  {
> -	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> +	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1] =
>  		"DP-HDMI ADAPTOR\x04";
>  
>  	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
> -		      sizeof(dp_dual_mode_hdmi_id)) == 0;
> +		      DP_DUAL_MODE_HDMI_ID_LEN+1) == 0;

and neither should this

>  }
>  
>  static bool is_type1_adaptor(uint8_t adaptor_id)
> 
> -- 
> Regards,
> Brahmajit

-- 
Ville Syrjälä
Intel

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Brahmajit]

On 01.10.2024 17:02, Ville Syrjälä wrote:
> > -static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> > +static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1])
> 
> This should not have the +1
> 
> >  {
> > -	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> > +	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1] =
> >  		"DP-HDMI ADAPTOR\x04";
> >  
> >  	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
> > -		      sizeof(dp_dual_mode_hdmi_id)) == 0;
> > +		      DP_DUAL_MODE_HDMI_ID_LEN+1) == 0;
> 
> and neither should this
> 
> >  }

Does this look good?

--- a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
@@ -160,11 +160,11 @@ EXPORT_SYMBOL(drm_dp_dual_mode_write);
 
 static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
 {
-	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
+	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN + 1] =
 		"DP-HDMI ADAPTOR\x04";
 
 	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
-		      sizeof(dp_dual_mode_hdmi_id)) == 0;
+		      DP_DUAL_MODE_HDMI_ID_LEN) == 0;
 }
 
 static bool is_type1_adaptor(uint8_t adaptor_id)
-- 
Regards,
Brahmajit

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Ville Syrjälä]

On Tue, Oct 01, 2024 at 05:02:15PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 01, 2024 at 07:18:23PM +0530, Brahmajit wrote:
> > On 01.10.2024 16:28, Ville Syrjälä wrote:
> > > Looks like gcc is warning about perfectly legal code
> > > (as far as the C standard goes).
> > > 
> > > But we could work around by adding room for the '\0' and
> > > s/sizeof(...)/DP_DUAL_MODE_HDMI_ID_LEN/ in the memcmp().
> > 
> > Sorry I'm new, but can we do something like this?
> > 
> > --- a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
> > +++ b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
> > @@ -158,13 +158,13 @@ ssize_t drm_dp_dual_mode_write(struct i2c_adapter *adapter,
> >  }
> >  EXPORT_SYMBOL(drm_dp_dual_mode_write);
> >  
> > -static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> > +static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1])
> 
> This should not have the +1
> 
> >  {
> > -	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =

Also we shuld probably remind why the +1 is there.
So a comment here would be good. 

Eg.
/* +1 to avoid spurious -W<whatever> warnings */

> > +	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1] =
> >  		"DP-HDMI ADAPTOR\x04";
> >  
> >  	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
> > -		      sizeof(dp_dual_mode_hdmi_id)) == 0;
> > +		      DP_DUAL_MODE_HDMI_ID_LEN+1) == 0;
> 
> and neither should this
> 
> >  }
> >  
> >  static bool is_type1_adaptor(uint8_t adaptor_id)
> > 
> > -- 
> > Regards,
> > Brahmajit
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Ville Syrjälä]

On Tue, Oct 01, 2024 at 07:36:17PM +0530, Brahmajit wrote:
> On 01.10.2024 17:02, Ville Syrjälä wrote:
> > > -static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
> > > +static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1])
> > 
> > This should not have the +1
> > 
> > >  {
> > > -	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> > > +	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN+1] =
> > >  		"DP-HDMI ADAPTOR\x04";
> > >  
> > >  	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
> > > -		      sizeof(dp_dual_mode_hdmi_id)) == 0;
> > > +		      DP_DUAL_MODE_HDMI_ID_LEN+1) == 0;
> > 
> > and neither should this
> > 
> > >  }
> 
> Does this look good?

Aye. Please add the small comment I mentioned, and post
to dri-devel@lists.freedesktop.org

> 
> --- a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
> @@ -160,11 +160,11 @@ EXPORT_SYMBOL(drm_dp_dual_mode_write);
>  
>  static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
>  {
> -	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
> +	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN + 1] =
>  		"DP-HDMI ADAPTOR\x04";
>  
>  	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
> -		      sizeof(dp_dual_mode_hdmi_id)) == 0;
> +		      DP_DUAL_MODE_HDMI_ID_LEN) == 0;
>  }
>  
>  static bool is_type1_adaptor(uint8_t adaptor_id)
> -- 
> Regards,
> Brahmajit

-- 
Ville Syrjälä
Intel

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Brahmajit]

On 01.10.2024 17:12, Ville Syrjälä wrote:
> Aye. Please add the small comment I mentioned, and post
> to dri-devel@lists.freedesktop.org
Sending patch to dri-devel@lists.freedesktop.org
-- 
Regards,
Brahmajit

Re: Build failure with GCC 15 in drivers/gpu/drm/display/drm_dp_dual_mode_helper.c

[Brahmajit]

I did sent the patch to dri-devel mailing list, Ville can you please
check, I'm not subscribed to the list (I'm subscribing now).

-- 
Regards,
Brahmajit

[PATCH 1/1] drm/display: Fix building with GCC 15

[Brahmajit Das]

GCC 15 enables -Werror=unterminated-string-initialization by default.
This results in the following build error

drivers/gpu/drm/display/drm_dp_dual_mode_helper.c: In function ‘is_hdmi_adaptor’:
drivers/gpu/drm/display/drm_dp_dual_mode_helper.c:164:17: error: initializer-string for array of
 ‘char’ is too long [-Werror=unterminated-string-initialization]
  164 |                 "DP-HDMI ADAPTOR\x04";
      |                 ^~~~~~~~~~~~~~~~~~~~~

After discussion with Ville, the fix was to increase the size of
dp_dual_mode_hdmi_id array by one, so that it can accommodate the new
line character. This should let us build the kernel with GCC 15.

Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
---
 drivers/gpu/drm/display/drm_dp_dual_mode_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
index 14a2a8473682..295375868db6 100644
--- a/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_dual_mode_helper.c
@@ -160,11 +160,12 @@ EXPORT_SYMBOL(drm_dp_dual_mode_write);
 
 static bool is_hdmi_adaptor(const char hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN])
 {
-	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN] =
+	//+1 to avaoid spurious -Werror=unterminated-string-initialization warning
+	static const char dp_dual_mode_hdmi_id[DP_DUAL_MODE_HDMI_ID_LEN + 1] =
 		"DP-HDMI ADAPTOR\x04";
 
 	return memcmp(hdmi_id, dp_dual_mode_hdmi_id,
-		      sizeof(dp_dual_mode_hdmi_id)) == 0;
+		      DP_DUAL_MODE_HDMI_ID_LEN) == 0;
 }
 
 static bool is_type1_adaptor(uint8_t adaptor_id)
-- 
2.46.2