On Wed, 15 Jan 2025 12:02:00 +0300 Dan Carpenter <dan.carpen...@linaro.org> wrote:
> You introduced a bug by changing the order. > > You have to undestand that to the original authors this stuff was really > easy and they knew the order of the struct members because they chose it > deliberately. In the end, they get so used to the code that > "&record->trace" just becomes an idiom for casting "record" and they > forget how it looks to a newcomer. > > I *personally* am not a fan of code which assumes we know the order of > the struct members so I don't have a problem with you re-writing the > code. But the commit message must say that it is just a cleanup and not > a fix. > > Which reminds me that I had intended to create a container_of_first() > for code like this which assumes that container_of() is just a cast. > There is lots of code like this: > > struct something *member = container_of(p, struct foo, first_member); > > if (IS_ERR(member)) { > > Which relies on the face that "first_member" is the first member of > foo struct. It's a quite common thing. I agree that this is not a bug, but I will happily take a clean up to make it more robust. -- Steve