On 22/11/2018 14:35, David Sterba wrote: > On Thu, Nov 22, 2018 at 02:15:28PM +0100, Johannes Thumshirn wrote: >> err holds the return value of either btrfs_del_root_ref() or >> btrfs_del_inode_ref() but it hasn't been checked since it's >> introduction with commit fe66a05a0679 (Btrfs: improve error handling >> for btrfs_insert_dir_item callers) in 2012. >> >> The first attempt in removing the variable was rejected, so instead of >> removing 'err', start handling the errors of >> btrfs_del_root_ref()/btrfs_del_inode_ref() by aborting the transaction in >> btrfs_add_link() directly instead of the call-sites. > > ... which is an anti-pattern. > > https://btrfs.wiki.kernel.org/index.php/Development_notes#Error_handling_and_transaction_abort > > In case there are several conditons that can fail we want to see the > first one for later analysis and debugging. OK, but then handling the error of either btrfs_del_root_ref() or btrfs_del_inode_ref() will essentially only be printing an error message as anything else would hide the error condition which has led us into this path (i.e. btrfs_insert_dir_item() returning EEXISTS or EOVERFLOW), wouldn't it? Byte, Johannes -- Johannes Thumshirn SUSE Labs jthumshirn@xxxxxxx +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
