When converting from int to string, we must allow for up to 10-chars (2147483647).

Fixes the following W=1 kernel build warning(s):

 drivers/gpu/drm/tegra/hub.c: In function ‘tegra_display_hub_probe’:
 drivers/gpu/drm/tegra/hub.c:1106:47: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
 drivers/gpu/drm/tegra/hub.c:1106:42: note: directive argument in the range [0, 4294967294]
 drivers/gpu/drm/tegra/hub.c:1106:17: note: ‘snprintf’ output between 6 and 15 bytes into a destination of size 8

Signed-off-by: Lee Jones <lee@kernel.org>
---
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-tegra@vger.kernel.org
---
 drivers/gpu/drm/tegra/hub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index 1af5f8318d914..f21e57e8599ee 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -1101,7 +1101,7 @@ static int tegra_display_hub_probe(struct platform_device *pdev)
 
 	for (i = 0; i < hub->soc->num_wgrps; i++) {
 		struct tegra_windowgroup *wgrp = &hub->wgrps[i];
-		char id[8];
+		char id[16];
 
 		snprintf(id, sizeof(id), "wgrp%u", i);
 		mutex_init(&wgrp->lock);
-- 
2.42.0.rc1.204.g551eb34607-goog

On Thu, Aug 24, 2023 at 08:37:00AM +0100, Lee Jones wrote:
> When converting from int to string, we must allow for up to 10-chars (2147483647).
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  drivers/gpu/drm/tegra/hub.c: In function ‘tegra_display_hub_probe’:
>  drivers/gpu/drm/tegra/hub.c:1106:47: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
>  drivers/gpu/drm/tegra/hub.c:1106:42: note: directive argument in the range [0, 4294967294]
>  drivers/gpu/drm/tegra/hub.c:1106:17: note: ‘snprintf’ output between 6 and 15 bytes into a destination of size 8

I wish there was (perhaps there is?) a better way to annotate that i
will always be within a given range. In practice this is always going to
be smaller than 10 and even in future hardware I wouldn't expect this to
ever exceed anything like 32 or 64, so 8 characters is already generous.

Thierry

> 
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Mikko Perttunen <mperttunen@nvidia.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Jonathan Hunter <jonathanh@nvidia.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: dri-devel@lists.freedesktop.org
> Cc: linux-tegra@vger.kernel.org
> ---
>  drivers/gpu/drm/tegra/hub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
> index 1af5f8318d914..f21e57e8599ee 100644
> --- a/drivers/gpu/drm/tegra/hub.c
> +++ b/drivers/gpu/drm/tegra/hub.c
> @@ -1101,7 +1101,7 @@ static int tegra_display_hub_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < hub->soc->num_wgrps; i++) {
>  		struct tegra_windowgroup *wgrp = &hub->wgrps[i];
> -		char id[8];
> +		char id[16];
>  
>  		snprintf(id, sizeof(id), "wgrp%u", i);
>  		mutex_init(&wgrp->lock);
> -- 
> 2.42.0.rc1.204.g551eb34607-goog
> 

On Thu, 24 Aug 2023, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Thu, Aug 24, 2023 at 08:37:00AM +0100, Lee Jones wrote:
>> When converting from int to string, we must allow for up to 10-chars (2147483647).
>> 
>> Fixes the following W=1 kernel build warning(s):
>> 
>>  drivers/gpu/drm/tegra/hub.c: In function ‘tegra_display_hub_probe’:
>>  drivers/gpu/drm/tegra/hub.c:1106:47: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
>>  drivers/gpu/drm/tegra/hub.c:1106:42: note: directive argument in the range [0, 4294967294]
>>  drivers/gpu/drm/tegra/hub.c:1106:17: note: ‘snprintf’ output between 6 and 15 bytes into a destination of size 8
>
> I wish there was (perhaps there is?) a better way to annotate that i
> will always be within a given range. In practice this is always going to
> be smaller than 10 and even in future hardware I wouldn't expect this to
> ever exceed anything like 32 or 64, so 8 characters is already generous.

I assume you could do

	snprintf(id, sizeof(id), "wgrp%u", (unsigned char)i);

but it's perhaps less obvious than just increasing the size of the
buffer.

BR,
Jani.

>
> Thierry
>
>> 
>> Signed-off-by: Lee Jones <lee@kernel.org>
>> ---
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Cc: Mikko Perttunen <mperttunen@nvidia.com>
>> Cc: David Airlie <airlied@gmail.com>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Jonathan Hunter <jonathanh@nvidia.com>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: linux-tegra@vger.kernel.org
>> ---
>>  drivers/gpu/drm/tegra/hub.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
>> index 1af5f8318d914..f21e57e8599ee 100644
>> --- a/drivers/gpu/drm/tegra/hub.c
>> +++ b/drivers/gpu/drm/tegra/hub.c
>> @@ -1101,7 +1101,7 @@ static int tegra_display_hub_probe(struct platform_device *pdev)
>>  
>>  	for (i = 0; i < hub->soc->num_wgrps; i++) {
>>  		struct tegra_windowgroup *wgrp = &hub->wgrps[i];
>> -		char id[8];
>> +		char id[16];
>>  
>>  		snprintf(id, sizeof(id), "wgrp%u", i);
>>  		mutex_init(&wgrp->lock);
>> -- 
>> 2.42.0.rc1.204.g551eb34607-goog
>> 

-- 
Jani Nikula, Intel Open Source Graphics Center

On Thu, 24 Aug 2023, Jani Nikula wrote:

> On Thu, 24 Aug 2023, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Thu, Aug 24, 2023 at 08:37:00AM +0100, Lee Jones wrote:
> >> When converting from int to string, we must allow for up to 10-chars (2147483647).
> >> 
> >> Fixes the following W=1 kernel build warning(s):
> >> 
> >>  drivers/gpu/drm/tegra/hub.c: In function ‘tegra_display_hub_probe’:
> >>  drivers/gpu/drm/tegra/hub.c:1106:47: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
> >>  drivers/gpu/drm/tegra/hub.c:1106:42: note: directive argument in the range [0, 4294967294]
> >>  drivers/gpu/drm/tegra/hub.c:1106:17: note: ‘snprintf’ output between 6 and 15 bytes into a destination of size 8
> >
> > I wish there was (perhaps there is?) a better way to annotate that i
> > will always be within a given range. In practice this is always going to
> > be smaller than 10 and even in future hardware I wouldn't expect this to
> > ever exceed anything like 32 or 64, so 8 characters is already generous.
> 
> I assume you could do
> 
> 	snprintf(id, sizeof(id), "wgrp%u", (unsigned char)i);
> 
> but it's perhaps less obvious than just increasing the size of the
> buffer.

I had the very same thought process.

-- 
Lee Jones [李琼斯]

On Thu, Aug 24, 2023 at 01:01:24PM +0100, Lee Jones wrote:
> On Thu, 24 Aug 2023, Jani Nikula wrote:
> 
> > On Thu, 24 Aug 2023, Thierry Reding <thierry.reding@gmail.com> wrote:
> > > On Thu, Aug 24, 2023 at 08:37:00AM +0100, Lee Jones wrote:
> > >> When converting from int to string, we must allow for up to 10-chars (2147483647).
> > >> 
> > >> Fixes the following W=1 kernel build warning(s):
> > >> 
> > >>  drivers/gpu/drm/tegra/hub.c: In function ‘tegra_display_hub_probe’:
> > >>  drivers/gpu/drm/tegra/hub.c:1106:47: warning: ‘%u’ directive output may be truncated writing between 1 and 10 bytes into a region of size 4 [-Wformat-truncation=]
> > >>  drivers/gpu/drm/tegra/hub.c:1106:42: note: directive argument in the range [0, 4294967294]
> > >>  drivers/gpu/drm/tegra/hub.c:1106:17: note: ‘snprintf’ output between 6 and 15 bytes into a destination of size 8
> > >
> > > I wish there was (perhaps there is?) a better way to annotate that i
> > > will always be within a given range. In practice this is always going to
> > > be smaller than 10 and even in future hardware I wouldn't expect this to
> > > ever exceed anything like 32 or 64, so 8 characters is already generous.
> > 
> > I assume you could do
> > 
> > 	snprintf(id, sizeof(id), "wgrp%u", (unsigned char)i);
> > 
> > but it's perhaps less obvious than just increasing the size of the
> > buffer.
> 
> I had the very same thought process.

It's not a big deal, this happens all on the stack, so adding a couple
of bytes isn't going to hurt very much. Still with all the tooling that
we have it'd be nice if something could be added. Perhaps an alternative
would be to reject values for num_wgrp that are bigger than 32. With
that the compiler might have enough information not to warn about this.

Anyway, no need to spend any more time on this, I'm fine with the patch
as-is.

Thierry