On Mon, Oct 14, 2024 at 10:15:25AM +0200, Javier Carrasco wrote: > On 14/10/2024 10:12, Dan Carpenter wrote: > > On Mon, Oct 14, 2024 at 09:59:49AM +0200, Javier Carrasco wrote: > >> This approach is great as long as the maintainer accepts mid-scope > >> variable declaration and the goto instructions get refactored, as stated > >> in cleanup.h. > >> > >> The first point is not being that problematic so far, but the second one > >> is trickier, and we all have to take special care to avoid such issues, > >> even if they don't look dangerous in the current code, because adding a > >> goto where there cleanup attribute is already used can be overlooked as > >> well. > >> > > > > To be honest, I don't really understand this paragraph. I think maybe you're > > talking about if we declare the variable at the top and forget to initialize it > > to NULL? It leads to an uninitialized variable if we exit the function before > > it is initialized. > > > > No, I am talking about declaring the variable mid-scope, and later on > adding a goto before that declaration in a different patch, let's say > far above the variable declaration. As soon as a goto is added, care > must be taken to make sure that we don't have variables with the cleanup > attribute in the scope. Just something to take into account. > Huh. That's an interesting point. If you have: if (ret) goto done; struct device_node *fw_node __free(device_node) = something; Then fw_node isn't initialized when we get to done. However, in my simple test this triggered a build failure with Clang so I believe we would catch this sort of bug pretty quickly. regards, dan carpenter
