On Tue, 2024-12-10 at 21:12 +0200, Alex Markuze wrote: > The main goal of this patch is to solve erroneous read sizes and > overflows. > The convoluted 'if else' chain is a recipe for disaster. Currently, > exec stops immediately on first ret that indicates an error. > If you have additional refactoring thoughts feel free to add more > patches, This is mainly a bug fix, that solves both the immediate > overflow bug and attempts to make this code more manageable to > mitigate future bugs. I see your point. I simply see several cases of ret > 0 check: https://elixir.bootlin.com/linux/v6.12.3/source/fs/ceph/file.c#L1150 https://elixir.bootlin.com/linux/v6.12.3/source/fs/ceph/file.c#L1158 https://elixir.bootlin.com/linux/v6.12.3/source/fs/ceph/file.c#L1163 <- your fix here https://elixir.bootlin.com/linux/v6.12.3/source/fs/ceph/file.c#L1192 <- your fix here too https://elixir.bootlin.com/linux/v6.12.3/source/fs/ceph/file.c#L1236 And there are places to check ret for negative values: https://elixir.bootlin.com/linux/v6.12.3/source/fs/ceph/file.c#L1160 https://elixir.bootlin.com/linux/v6.12.3/source/fs/ceph/file.c#L1226 These checks distributes in the function's code, it could be confusing and, potentially, be the source of new bugs during modifications. I simply have feelings that this logic somehow requires refactoring to improve the execution flow. But if you would like not to do it, then I am OK with it. Thanks, Slava.
