On 05/12/2025 10:48, Ard Biesheuvel wrote:
On Sun, 9 Nov 2025 at 19:00, Ard Biesheuvel <[email protected]> wrote:
On Sat, 8 Nov 2025 at 01:27, Tvrtko Ursulin <[email protected]> wrote:
On 07/11/2025 16:42, Ard Biesheuvel wrote:
From: Ard Biesheuvel <[email protected]>
GCC notices that the 16-byte uabi_name field could theoretically be too
small for the formatted string if the instance number exceeds 100.
Given that there are apparently ABI concerns here, this is the minimal
fix that shuts up the compiler without changing the output or the
maximum length for existing values < 100.
What would be those ABI concerns? I don't immediately see any.
drivers/gpu/drm/i915/intel_memory_region.c: In function
‘intel_memory_region_create’:
drivers/gpu/drm/i915/intel_memory_region.c:273:61: error: ‘%u’ directive output
may be truncated writing between 1 and 5 bytes into a region of size between 3
and 11 [-Werror=format-truncation=]
273 | snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
| ^~
drivers/gpu/drm/i915/intel_memory_region.c:273:58: note: directive argument in
the range [0, 65535]
273 | snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
| ^~~~~~
drivers/gpu/drm/i915/intel_memory_region.c:273:9: note: ‘snprintf’ output
between 7 and 19 bytes into a destination of size 16
273 | snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
274 | intel_memory_type_str(type), instance);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Ard Biesheuvel <[email protected]>
---
Cc: Jani Nikula <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Cc: Rodrigo Vivi <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Simona Vetter <[email protected]>
Cc: [email protected]
Cc: [email protected]
This is unlikely to be the right fix, but sending a wrong patch is
usually a better way to elicit a response than just sending a bug
report.
drivers/gpu/drm/i915/intel_memory_region.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_memory_region.c
b/drivers/gpu/drm/i915/intel_memory_region.c
index 59bd603e6deb..ad4afcf0c58a 100644
--- a/drivers/gpu/drm/i915/intel_memory_region.c
+++ b/drivers/gpu/drm/i915/intel_memory_region.c
@@ -271,7 +271,7 @@ intel_memory_region_create(struct drm_i915_private *i915,
mem->instance = instance;
snprintf(mem->uabi_name, sizeof(mem->uabi_name), "%s%u",
- intel_memory_type_str(type), instance);
+ intel_memory_type_str(type), instance % 100);
It's a theoretical issue only since there is no hardware with a double
digit number of instances.
But I guess much prettier fix would be to simply grow the buffer.
OK, so something like
--- a/drivers/gpu/drm/i915/intel_memory_region.h
+++ b/drivers/gpu/drm/i915/intel_memory_region.h
@@ -72,7 +72,7 @@ struct intel_memory_region {
u16 instance;
enum intel_region_id id;
char name[16];
- char uabi_name[16];
+ char uabi_name[20];
bool private; /* not for userspace */
struct {
Yes please. There is only two of those objects at majority of systems,
and 3-4 on a few discrete cards supported by i915, so no big deal to
grow them a tiny bit.
Also, hm, how come gcc does not find the mem->name vsnprintf from
intel_memory_region_set_name?
AFAICT, intel_memory_region_set_name() is never called with a format
string that could produce more than 15/16 bytes of output.
Right, I reminded myself that the non-uabi visible name does not have
the instance number in it.
Regards,
Tvrtko
