On Mon, Jan 07, 2019 at 04:22:01PM +0100, Johannes Thumshirn wrote: > On 02/01/2019 18:43, Nikolay Borisov wrote: > > On 2.01.19 г. 19:05 ч., David Sterba wrote: > >> This is repeating code and could be simplified ... to the original code. > > > > It does and this is intentional. What I've strived to do is make the > > idea of the code obvious and not try to reduce the total line of code. > > It is a massive improvement given the code which modifies extent_end > > triggers only in case ret is > 0. I discussed this with Johannes and he > > agreed with my assessment. > > > >> I'm not sure this patch is an improvement. > > I think it really helps when trying to understand the code. > > This is why the patch was created in the first place, I asked Nik about > something and we went over the code together. I've tried to read the new version again with fresh eyes and still don't see any improvement in readability. Not that the whole function is easy to follow, quite the opposite, but I'd rather see continued refactoring that untangles the gotos. The return codes of btrfs_lookup_file_extent follow the common btrfs_search_slot pattern < 0 error, 0 found, > 0 not found. And in the last case there's an adjustment needed. Read the item type, check and branch if necessary. No surprises there. The amount of duplicated code is also not something trivial like a variable asisgnment, there are arguments passed etc.
