Skip to content

Fix error: '%s' directive output may be truncated#8658

Merged
davids5 merged 1 commit intoapache:masterfrom
xiaoxiang781216:warn
Mar 3, 2023
Merged

Fix error: '%s' directive output may be truncated#8658
davids5 merged 1 commit intoapache:masterfrom
xiaoxiang781216:warn

Conversation

@xiaoxiang781216
Copy link
Copy Markdown
Contributor

Summary

writing up to 31 bytes into a region of size 27 [-Werror=format-truncation=]

Impact

gpio driver

Testing

CI

@pkarashchenko
Copy link
Copy Markdown
Contributor

Are there any limitations for pinname string length? I mean, I do not fully understand the fix.

@xiaoxiang781216
Copy link
Copy Markdown
Contributor Author

Are there any limitations for pinname string length? I mean, I do not fully understand the fix.

gpio_pin_register defines a local char array with 32bytes and forward this array to gpio_pin_register_byname, so compiler assume that pinname may have 32bytes long in extreme case.

Copy link
Copy Markdown
Contributor

@pkarashchenko pkarashchenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like an increase to 40 should be enough, but we can go with 64 bytes as well

@hartmannathan
Copy link
Copy Markdown
Contributor

The only bad thing about increasing the sizes of stacked strings is that eventually we have to increase task stack sizes too, or we will face stack overflows. I don't know what to suggest, though. Heap allocation will introduce overhead and could contribute to memory fragmentation on systems with tiny memory resources. A static global scratch space is not re-entrant. Ever-growing stack footprints cause a cascade effect where we have to grow the stack sizes of our threads.

@pkarashchenko
Copy link
Copy Markdown
Contributor

Yes. Maybe we can decrease buffer size in gpio_pin_register instead of increasing it in gpio_pin_register_byname?

@hartmannathan
Copy link
Copy Markdown
Contributor

Yes. Maybe we can decrease buffer size in gpio_pin_register instead of increasing it in gpio_pin_register_byname?

Right. How many GPIO pins could we possibly have? If the MCU is a 212-pin BGA you'll have 3 digits; that will support up to 999 GPIO pins. We could splurge and say 4 digits ("gpioXXXX"), allowing for some obscenely large SoCs. With trailing NUL this is 9 chars, and "/dev/gpioXXXX" with trailing NUL is total of 14 characters. We can round up to 16. This solves problem and reduces stack usage instead of increasing it.

writing up to 31 bytes into a region of size 27 [-Werror=format-truncation=]

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
@xiaoxiang781216
Copy link
Copy Markdown
Contributor Author

Done.

Copy link
Copy Markdown
Contributor

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Logic is sound. 999 gpios on the soc, 999 gpios, take one down....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants