Looks to me it's because we unref the info here;

  /* Everything else should just fall back on a regular copy. */
  else
    g_object_unref (info);

...

then:

      result = btrfs_reflink_with_progress (in, out, info, cancellable,

But info is freed.  Needs some refactoring to preserve info until at least that point.  I suggest changing the function to "goto out" style with cleanups at the end.

Created attachment 234419 [details] [review]
0001-GFile-Clean-up-file_copy_fallback-to-fix-SEGV-with-b.patch

Comment on attachment 234419 [details] [review]
0001-GFile-Clean-up-file_copy_fallback-to-fix-SEGV-with-b.patch

>+  if (!out)
>+    goto out;

Ick.

> #ifdef HAVE_SYS_IOCTL_H
>-  if (G_IS_FILE_DESCRIPTOR_BASED (in) && G_IS_FILE_DESCRIPTOR_BASED (out))
>+  if (!did_optimized_copy && G_IS_FILE_DESCRIPTOR_BASED (in) && G_IS_FILE_DESCRIPTOR_BASED (out))

did_optimized_copy can't actually be TRUE here. (And if you're thinking "well maybe we'll add another case before this one later", the flip side of that is that clang and coverity will probably complain about it.)

And anyway, I think it was clearer with the gotos than with the if !did_optimized_copy. If you had each of the the three copying cases do "ret = TRUE; goto out;" if they succeeded, then you could move the final g_file_copy_attributes() to after out:, but only calling it if ret==TRUE (and then you don't need to call it from copy_symlink()).

Created attachment 234434 [details] [review]
0001-GFile-Clean-up-file_copy_fallback-to-fix-SEGV-with-b.patch

Yeah, maybe in this case it's slightly clearer.

(In reply to comment #4)
> Created an attachment (id=234434) [details] [review]
> 0001-GFile-Clean-up-file_copy_fallback-to-fix-SEGV-with-b.patch
> 

I can't find anything wrong with the patch itself (and it passes make check for me). However, maybe we should add a test for this problem to gfile? The old code didn't trigger any test failures either.

Also, a little nitpick: personally, I think two labels, "err" and "out" would be clearer than one label "out" + ret = FALSE/TRUE. That makes it clear to anyone reading the code whether the premature exit is a failure condition or not. So, something like:

[…]
out:
  /* Ignore errors here. Failure to copy metadata is not a hard error */
  (void) g_file_copy_attributes (source, destination,
                                 flags, cancellable, NULL);

  ret = TRUE;
err:
  if (in)
    {
      /* Don't care about errors in source here */
      (void) g_input_stream_close (in, cancellable, NULL);
      g_object_unref (in);
    }
[…]

And then, replace "ret = TRUE; goto out;" with "goto out;", and plain "goto out;" with "goto err;". But this is just my opinion. :)

Comment on attachment 234434 [details] [review]
0001-GFile-Clean-up-file_copy_fallback-to-fix-SEGV-with-b.patch

yeah, works for me

(In reply to comment #5)
> I can't find anything wrong with the patch itself (and it passes make check for
> me). However, maybe we should add a test for this problem to gfile? The old
> code didn't trigger any test failures either.

You can only test it if you have a btrfs file system though...

> (In reply to comment #5)
> > I can't find anything wrong with the patch itself (and it passes make check for
> > me). However, maybe we should add a test for this problem to gfile? The old
> > code didn't trigger any test failures either.
> 
> You can only test it if you have a btrfs file system though...

That's the thing. I did run the gfile tests on a btrfs filesystem in the original bug 626497, and they all passed. You can try this yourself by loopback-mounting a btrfs filesystem, and running `gio/tests/live-g-file` inside it.