Re: [PATCH v2] btrfs: iterate over unused chunk space in FITRIM

[Omar Sandoval <osandov@osandov.com>]

On Fri, May 08, 2015 at 06:17:26PM -0400, Jeff Mahoney wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 5/8/15 4:38 PM, Filipe David Manana wrote:
> > On Tue, May 5, 2015 at 7:19 PM, Jeff Mahoney <jeffm@suse.com>
> > wrote:
> >> -static int contains_pending_extent(struct btrfs_trans_handle
> >> *trans, +static int contains_pending_extent(struct
> >> btrfs_transaction *transaction, struct btrfs_device *device, u64
> >> *start, u64 len) { +       struct btrfs_fs_info *fs_info =
> >> device->dev_root->fs_info; struct extent_map *em; -       struct
> >> list_head *search_list = &trans->transaction->pending_chunks; +
> >> struct list_head *search_list = &transaction->pending_chunks;
> > 
> > transaction can be NULL so we need to check for that first (as you
> > do below).
> 
> The ordering here is safe. We're not dereferencing the transaction,
> we're getting the address of one of the members. search_list will
> point to 0x100 or something. We don't ever dereference that without
> checking the transaction against NULL.
> 

Hi, Jeff,

I believe I've seen this topic come up before -- see [1]. Although it's
probably fine on any sane compiler, it's apparently not correct from a
language standard standpoint. Probably better safe than sorry.

[1] https://software.intel.com/en-us/blogs/2015/04/20/null-pointer-dereferencing-causes-undefined-behavior

Thanks!
-- 
Omar