On 22.02.19 г. 12:16 ч., Qu Wenruo wrote:
> This patchset can be fetched from github:
> https://github.com/adam900710/linux/tree/cleanup_alloc_extent_buffer
> Which is based on v5.0-rc7
>
> There are 5 extent buffer alloc functions in btrfs:
> __alloc_extent_buffer();
> alloc_extent_buffer();
> __alloc_dummy_extent_buffer();
> alloc_dummy_extent_buffer();
> alloc_test_extent_buffer();
>
> However their return value is not unified for failure mode:
> __alloc_extent_buffer() Never fail
> alloc_extent_buffer() PTR_ERR()
> __alloc_dummy_extent_buffer() NULL
This function can never return NULL, if __alloc_extent_buffer cannot
fail then the only error this function returns is ERR_PTR(ENOMEM);
> alloc_dummy_extent_buffer() NULL
Same thing applies to this function
> alloc_test_extent_buffer() NULL
Same thing for this function, if we return exists then we must have
found it by find_extent_buffer hence it cannot be null. Otherwise we
return eb as allocated from alloc_dummy_extent_buffer. So how can null
be returned?
To me it really seems none of the function could return a NULL value, no?
>
> This causes some wrapper function to have 2 failure modes, like
> btrfs_find_create_tree_block() can return NULL or PTR_ERR(-ENOMEM) for
> its failure.
>
> This inconsistent behavior is making static checker and reader crazy.
>
> This patchset will unify the failure more of above 5 functions to
> PTR_ERR().
>
> Qu Wenruo (5):
> btrfs: extent_io: Add comment about the return value of
> alloc_extent_buffer()
> btrfs: extent_io: Unify the return value of __alloc_extent_buffer()
> with alloc_extent_buffer()
> btrfs: extent_io: Unify the return value of
> alloc_dummy_extent_buffer() with alloc_extent_buffer()
> btrfs: extent_io: Unify the return value of alloc_test_extent_buffer()
> with alloc_extent_buffer()
> btrfs: extent_io: Unify the return value of
> btrfs_clone_extent_buffer() with alloc_extent_buffer()
>
> fs/btrfs/backref.c | 8 ++--
> fs/btrfs/ctree.c | 16 ++++----
> fs/btrfs/extent_io.c | 56 +++++++++++++++++---------
> fs/btrfs/qgroup.c | 5 ++-
> fs/btrfs/tests/extent-buffer-tests.c | 6 ++-
> fs/btrfs/tests/extent-io-tests.c | 4 +-
> fs/btrfs/tests/free-space-tree-tests.c | 3 +-
> fs/btrfs/tests/inode-tests.c | 6 ++-
> fs/btrfs/tests/qgroup-tests.c | 3 +-
> 9 files changed, 68 insertions(+), 39 deletions(-)
>
