Skip to content

Add printf format attribute and const qualifier to _s_out#3

Closed
assisted-by-ai wants to merge 3 commits intoKicksecure:masterfrom
assisted-by-ai:claude/security-review-EYFzv
Closed

Add printf format attribute and const qualifier to _s_out#3
assisted-by-ai wants to merge 3 commits intoKicksecure:masterfrom
assisted-by-ai:claude/security-review-EYFzv

Conversation

@assisted-by-ai
Copy link
Copy Markdown

Summary

Improved type safety and compiler checking for the _s_out function by adding a printf format attribute and const qualifier to the format string parameter.

Key Changes

  • Added __printf(2, 3) attribute to the _s_out function declaration and definition to enable compiler format string validation
  • Changed the fmt parameter from char * to const char * to properly indicate that the format string is not modified

Implementation Details

These changes enable the compiler to:

  • Validate that format specifiers in the format string match the provided arguments
  • Warn about potential format string vulnerabilities
  • Enforce const-correctness for the format string parameter, which should never be modified by the function

This is a non-functional change that improves code quality and catches potential bugs at compile time.

https://claude.ai/code/session_018RqemCaZDrhmA7jV2eBgQ3

Annotate _s_out() with __printf(2, 3) so the compiler warns on format
string mismatches. Also mark fmt parameter as const since it is not
modified.

https://claude.ai/code/session_018RqemCaZDrhmA7jV2eBgQ3
@ArrayBolt3
Copy link
Copy Markdown

I don't have a problem with the concept behind this code, but given the small size and limited scope of tirdad, I don't expect it to get big enough for this to be very useful. We'd probably want to upstream this if we were to integrate it, and given how small and arguably "pedantic" it is, I don't know if we should bother upstream with this. (I'm also not sure the syntax Claude is using is correct.)

@adrelanos
Copy link
Copy Markdown
Member

Thoughts on this one? @0xsirus

@assisted-by-ai
Copy link
Copy Markdown
Author

The syntax is correct. __printf(a, b) is the kernel's portable macro for __attribute__((__format__(printf, a, b))), defined in include/linux/compiler_attributes.h (search for __printf). It's used extensively throughout the kernel — for example in include/linux/dev_printk.h and include/linux/printk.h.

That said, I agree the practical value is minimal here — there are only 5 call sites, all with string literals and no format arguments. Happy to close this if you'd prefer not to upstream it.

@0xsirus
Copy link
Copy Markdown

0xsirus commented Apr 7, 2026

The existing print function is fine and the suggested change is not necessary.

claude added 2 commits April 10, 2026 10:20
The Kicksecure#3 maintainer feedback is that the __printf
attribute change is too pedantic for a module this small. Record this
so future agents don't re-propose it.

https://claude.ai/code/session_018RqemCaZDrhmA7jV2eBgQ3
@adrelanos
Copy link
Copy Markdown
Member

superseded by assisted-by-ai#4

@adrelanos adrelanos closed this Apr 10, 2026
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