docs: deprecated.rst: Add note to the use of struct_size() helper
Subsystems (2)
PATCH 1/1 docs: deprecated.rst: Add note to the use of struct_size() helper
Add a note to educate people about the proper use of struct_size() when the trailing array in the enclosing structure is a one-element array.
Zero-length and one-element arrays will soon be removed from the kernel, but in the meantime, it's worth letting people know how to correctly use struct_size() together with such constructs.
---
Kees,
This is not substitute for the patch I'll write about flexible-arrays and the deprecation of zero-lenght and one-element arrays.
Thanks
--
Gustavo
Documentation/process/deprecated.rst | 11 +++++++++++ 1 file changed, 11 insertions(+)
Documentation/process/deprecated.rst +11 -0
@@ -85,6 +85,17 @@ Instead, use the helper:: header = kzalloc(struct_size(header, item, count), GFP_KERNEL); +NOTE: You might want to use the following form in case the trailing array +is a one-element array, as unlike zero-length arrays and flexible-array +members, `one-element arrays occupy at least as much space as a single +object of the type <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, +hence they contribute to the size of the enclosing structure:: + + header = kzalloc(struct_size(header, item, count - 1), GFP_KERNEL); + +It's also worth noting that one-element arrays --together with zero-length +arrays-- will soon be completely removed from the codebase and deprecated. + See array_size(), array3_size(), and struct_size(), for more details as well as the related check_add_overflow() and check_mul_overflow() family of functions.
Signature · 1 line
2.27.0
Quoted 7 lines · Gustavo A. R. Silva
Add a note to educate people about the proper use of
struct_size()when the trailing array in the enclosing structure is a one-element array.Zero-length and one-element arrays will soon be removed from the kernel, but in the meantime, it's worth letting people know how to correctly use
struct_size()together with such constructs.
Quoted 7 lines
Signed-off-by Gustavo A. R. Silva <[email protected]>---
Kees,
This is not substitute for the patch I'll write about flexible-arrays and the deprecation of zero-lenght and one-element arrays.
Hm, hm. I think I'd rather just get the 0/1-array docs written, since that will mean this paragraph isn't needed at all. (Or rather, it can be modified to say "if you're using struct_size() on a 1-array, stop using a 1-array, see [link]". If someone is going to switch around their code, they need to switch to flex at the same time, IMO.
-Kees
Quoted 33 lines
Thanks
--
GustavoDocumentation/process/deprecated.rst | 11 +++++++++++ 1 file changed, 11 insertions(+)Documentation/process/deprecated.rst +11
-0@@ -85,6 +85,17 @@ Instead, use the helper:: header = kzalloc(struct_size(header, item, count), GFP_KERNEL); +NOTE: You might want to use the following form in case the trailing array +is a one-element array, as unlike zero-length arrays and flexible-array +members, `one-element arrays occupy at least as much space as a single +object of the type <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, +hence they contribute to the size of the enclosing structure:: + + header = kzalloc(struct_size(header, item, count - 1), GFP_KERNEL); + +It's also worth noting that one-element arrays --together with zero-length +arrays-- will soon be completely removed from the codebase and deprecated. + See array_size(), array3_size(), and struct_size(), for more details as well as the related check_add_overflow() and check_mul_overflow() family of functions.Signature · 1 line
2.27.0
Signature · 1 line
Kees Cook
Quoted 11 lines · Kees Cook
Quoted 10 lines · Gustavo A. R. Silva
Add a note to educate people about the proper use of
struct_size()when the trailing array in the enclosing structure is a one-element array.Zero-length and one-element arrays will soon be removed from the kernel, but in the meantime, it's worth letting people know how to correctly use
struct_size()together with such constructs.Signed-off-by Gustavo A. R. Silva <[email protected]>---
Quoted 7 lines
Quoted 4 lines
Kees,
This is not substitute for the patch I'll write about flexible-arrays and the deprecation of zero-lenght and one-element arrays.
Hm, hm. I think I'd rather just get the 0/1-array docs written, since that will mean this paragraph isn't needed at all. (Or rather, it can be
Yeah. My reasoning for is that it will take a while --at least one development cycle more-- to completely get rid of all the 0/1-arrays. Also, this was motivated by the following comments from Christian König:
"May I suggest that we add a section how to correctly do this to Documentation/process/coding-style.rst or similar document?
I've seen a bunch of different approaches and some even doesn't work with some gcc versions and result in a broken binary."[1]
Quoted 4 lines
modified to say "if you're using
struct_size()on a 1-array, stop using a 1-array, see [link]". If someone is going to switch around their code, they need to switch to flex at the same time, IMO.
I agree with this. I can add the comments in quotes you suggest to this patch.
But I think we can add this note while I continue working on the flexible-array conversions. Once that work is complete, I can go back and update the documentation. :)
What do you think?
Thanks
--
Gustavo
[1] https://lore.kernel.org/lkml/[email protected]/
Quoted 32 lines
Quoted 29 lines
Documentation/process/deprecated.rst | 11 +++++++++++ 1 file changed, 11 insertions(+)Documentation/process/deprecated.rst +11
-0@@ -85,6 +85,17 @@ Instead, use the helper:: header = kzalloc(struct_size(header, item, count), GFP_KERNEL); +NOTE: You might want to use the following form in case the trailing array +is a one-element array, as unlike zero-length arrays and flexible-array +members, `one-element arrays occupy at least as much space as a single +object of the type <https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html>`_, +hence they contribute to the size of the enclosing structure:: + + header = kzalloc(struct_size(header, item, count - 1), GFP_KERNEL); + +It's also worth noting that one-element arrays --together with zero-length +arrays-- will soon be completely removed from the codebase and deprecated. + See array_size(), array3_size(), and struct_size(), for more details as well as the related check_add_overflow() and check_mul_overflow() family of functions.Signature · 1 line
2.27.0Signature · 1 line
Kees Cook
Quoted 2 lines · Gustavo A. R. Silva
Yeah. My reasoning for is that it will take a while --at least one development cycle more-- to completely get rid of all the 0/1-arrays.
Right -- but we need a place to point people when we tell them "please don't use 0-byte and 1-byte arrays", and the deprecated.rst is the place for that.
Having it in deprecated.rst once they're all gone only serves to explain why various compiler flags are enabled, etc. But while they're being removed, it serves as a single place to document the issue (as in, much of the flex-array patch commit log "boilerplate" can actually be repeated in deprecated.rst.
Quoted 5 lines
But I think we can add this note while I continue working on the flexible-array conversions. Once that work is complete, I can go back and update the documentation. :)
What do you think?
I think we need to document it at the beginning of the work (and I should have asked for this earlier). So let's just add a new section on dynamic array usage, etc. It can include a note about struct_size() as an example for why 1-byte arrays are so weird. :)
Signature · 1 line
Kees Cook
Quoted 18 lines · Kees Cook
Quoted 2 lines · Gustavo A. R. Silva
Yeah. My reasoning for is that it will take a while --at least one development cycle more-- to completely get rid of all the 0/1-arrays.
Right -- but we need a place to point people when we tell them "please don't use 0-byte and 1-byte arrays", and the
deprecated.rstis the place for that.Having it in
deprecated.rstonce they're all gone only serves to explain why various compiler flags are enabled, etc. But while they're being removed, it serves as a single place to document the issue (as in, much of the flex-array patch commit log "boilerplate" can actually be repeated indeprecated.rst.Quoted 4 lines
But I think we can add this note while I continue working on the flexible-array conversions. Once that work is complete, I can go back and update the documentation. :)
Quoted 7 lines
Quoted 1 line
What do you think?
I think we need to document it at the beginning of the work (and I should have asked for this earlier). So let's just add a new section on dynamic array usage, etc. It can include a note about
struct_size()as an example for why 1-byte arrays are so weird. :)
Got ya. :)
One last thing... I was thinking on adding such section (dynamic array usage) to coding-style.rst, explaining how to use struct_size() and transform the different open-coded versions we currently have in the kernel, e.g. I have seen people use offsetof() --and sometimes open-coded versions of sizeof_field()-- and its open-coded version to do arithmetic in allocator arguments.
Thanks
--
Gustavo
Quoted 27 lines · Gustavo A. R. Silva
Quoted 18 lines · Kees Cook
Quoted 2 lines · Gustavo A. R. Silva
Yeah. My reasoning for is that it will take a while --at least one development cycle more-- to completely get rid of all the 0/1-arrays.
Right -- but we need a place to point people when we tell them "please don't use 0-byte and 1-byte arrays", and the
deprecated.rstis the place for that.Having it in
deprecated.rstonce they're all gone only serves to explain why various compiler flags are enabled, etc. But while they're being removed, it serves as a single place to document the issue (as in, much of the flex-array patch commit log "boilerplate" can actually be repeated indeprecated.rst.Quoted 4 lines
But I think we can add this note while I continue working on the flexible-array conversions. Once that work is complete, I can go back and update the documentation. :)
Quoted 7 lines
Quoted 1 line
What do you think?
I think we need to document it at the beginning of the work (and I should have asked for this earlier). So let's just add a new section on dynamic array usage, etc. It can include a note about
struct_size()as an example for why 1-byte arrays are so weird. :)
Quoted 8 lines
Got ya. :)
One last thing... I was thinking on adding such section (dynamic array usage) to
coding-style.rst, explaining how to usestruct_size()and transform the different open-coded versions we currently have in the kernel, e.g. I have seen people useoffsetof()--and sometimes open-coded versions of sizeof_field()-- and its open-coded version to do arithmetic in allocator arguments.
Yeah, that sounds good to me!
-Kees
Quoted 5 lines
Thanks
--
Gustavo
Signature · 1 line
Kees Cook