On 29/01/25 16:24, Greg KH wrote:
On Wed, Jan 29, 2025 at 03:56:01PM +1030, Gustavo A. R. Silva wrote:
This is like container_of_const() but it contains an assert to
ensure that it's using the first member in the structure.
But why? If you "know" it's the first member, just do a normal cast.
If you don't, then you probably shouldn't be caring about this anyway,
right?
This is more about the cases where the member _must_ be first in the
structure. See below for an example related to -Wflex-array-member-not-at-end
Co-developed-by: Dan Carpenter <[email protected]>
Signed-off-by: Dan Carpenter <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
I will be using this in my -Wflex-array-member-not-at-end patches. :)
Confused, I'd like to see some users first please.
When addressing the -Wflex-array-member-not-at-end warnings, the common
scenario is when we have to separate the flexible-array member from the
rest of the members in the flexible structure, as shown below [1]:
struct bplus_header
{
struct_group_tagged(bplus_header_fixed, __hdr,
u8 flags; /* bit 0 - high bit of first free entry
offset
bit 5 - we're pointed to by an fnode,
the data btree or some ea or the
main ea bootage pointer ea_secno
bit 6 - suggest binary search
(unused)
bit 7 - 1 -> (internal) tree of
anodes
0 -> (leaf) list of extents */
u8 fill[3];
u8 n_free_nodes; /* free nodes in following array */
u8 n_used_nodes; /* used nodes in following array */
__le16 first_free; /* offset from start of header to
first free node in array */
);
union {
/* (internal) 2-word entries giving subtree pointers */
DECLARE_FLEX_ARRAY(struct bplus_internal_node, internal);
/* (external) 3-word entries giving sector runs */
DECLARE_FLEX_ARRAY(struct bplus_leaf_node, external);
} u;
};
struct_group_tagged() creates a new type: `struct bplus_header_fixed`, we
then use the newly created type to change the type of the middle objects
causing the warnings, and with that the warnings are gone:
@@ -453,7 +455,7 @@ struct fnode
__le16 flags; /* bit 1 set -> ea_secno is an
anode */
/* bit 8 set -> directory. first &
only extent
points to dnode. */
- struct bplus_header btree; /* b+ tree, 8 extents or 12 subtrees */
+ struct bplus_header_fixed btree; /* b+ tree, 8 extents or 12 subtrees */
union {
struct bplus_leaf_node external[8];
struct bplus_internal_node internal[12];
@@ -495,7 +497,7 @@ struct anode
__le32 self; /* pointer to this anode */
__le32 up; /* parent anode or fnode */
- struct bplus_header btree; /* b+tree, 40 extents or 60 subtrees */
+ struct bplus_header_fixed btree; /* b+tree, 40 extents or 60 subtrees */
union {
struct bplus_leaf_node external[40];
struct bplus_internal_node internal[60];
However, this newly created type, or rather the member `__hdr` (also
created when calling struct_group_tagged()) _must_ always be the first
member in the flexible struct `struct bplus_header`.
Then we need to use container_first() to retrieve a pointer to the flexible
structure [2], via which we can access the flexible-array member when
necessary:
diff --git a/fs/hpfs/anode.c b/fs/hpfs/anode.c
index c14c9a035ee0c0..a366f6ac71e436 100644
--- a/fs/hpfs/anode.c
+++ b/fs/hpfs/anode.c
@@ -27,7 +27,7 @@ secno hpfs_bplus_lookup(struct super_block *s, struct inode
*inode,
a = le32_to_cpu(btree->u.internal[i].down);
brelse(bh);
if (!(anode = hpfs_map_anode(s, a, &bh)))
return -1;
- btree = &anode->btree;
+ btree = container_first(&anode->btree, struct
bplus_header, __hdr);
goto go_down;
}
hpfs_error(s, "sector %08x not found in internal anode %08x",
sec, a);
@@ -69,12 +69,16 @@ secno hpfs_add_sector_to_btree(struct super_block *s, secno
node, int fnod, unsi
int n;
unsigned fs;
int c1, c2 = 0;
+ struct bplus_header *anode_btree = container_first(&anode->btree,
struct bplus_header, __hdr);
+ struct bplus_header *ranode_btree = container_first(&ranode->btree,
struct bplus_header, __hdr);
+ struct bplus_header *fnode_btree = container_first(&fnode->btree,
struct bplus_header, __hdr);
+
So, if we use container_first() (or container_of_first(), you tell
which name you prefer), we are asserting that that `__hdr` member
is always the first member in `struct bplus_header`.
I've explained all this at Plumbers last year. In any case, let
me know if you need more clarification. :)
Thanks!
--
Gustavo
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/diff/fs/hpfs/hpfs.h?h=testing/wfamnae-next20250124&id=f66219294267a2fba220f4f3118e11c5cda63d0b
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/diff/fs/hpfs/anode.c?h=testing/wfamnae-next20250124&id=f66219294267a2fba220f4f3118e11c5cda63d0b
