Don't hide ffi-inliner errors

[sj26]

If ffi-inliner isn't installed the earlier lines of these methods will raise an error and the Archive::Stat module won't be installed, so Archive::Stat.ffi_libarchive_free_stat doens't exist and also raises an error during the ensure block, hiding the originally-raised error.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

• I have read the CONTRIBUTING document.
• I have run the pre-merge tests locally and they pass.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
  • Not easily possible.
• All new and existing tests passed.
• All commits have been signed-off for the Developer Certificate of Origin.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[sj26]

Re:

ffi-libarchive/lib/ffi-libarchive/entry.rb

Line 177 in dae72e2

# TODO: get this work without ffi-inliner

I've been playing around — perhaps there is a way to have a minimal extension like this which can reach into File::Stat and avoid the need for ffi-inliner?

https://github.com/sj26/fiddle-file-stat

(I've done it in Fiddle, but the same should apply for FFI.)

[jaymzh]

Maybe I'm wrong, but won't this still be the same? It'll still raise an error, the ensure will still fire, the free_stat will still fire, no?

[phil@rider ~]$ cat /tmp/test2.rb 
#!/usr/bin/ruby


def test
  puts "foo"
  begin
    raise 'test error'
  ensure
    raise 'ensure error'
  end
end

test
[phil@rider ~]$ /tmp/test.rb 
foo
/tmp/test.rb:8:in `ensure in test': ensure error (RuntimeError)
	from /tmp/test.rb:8:in `test'
	from /tmp/test.rb:11:in `<main>'
/tmp/test.rb:6:in `test': test error (RuntimeError)
	from /tmp/test.rb:11:in `<main>'
[phil@rider ~]$ /tmp/test2.rb 
foo
/tmp/test2.rb:9:in `ensure in test': ensure error (RuntimeError)
	from /tmp/test2.rb:9:in `test'
	from /tmp/test2.rb:13:in `<main>'
/tmp/test2.rb:7:in `test': test error (RuntimeError)
	from /tmp/test2.rb:13:in `<main>'

I think you want to rescue the specific error you're looking for.

Or perhaps you want to not call ffi_libarchive_free_stat if stat is null?

[sj26]

Ah, sorry, I didn't explain very well. The error comes from the require on line 179, while loading the file and running ffi-inliner. That is caught a another error raised by 181. Then the ensure block runs line 189 which has no hope of succeeding.

It's closer to:

def test
  begin
    raise "original error"
  rescue
    raise "had an error"
  end
  foo = true
ensure
  unless foo
    raise "unrelated error"
  end
end

test

/tmp/test.rb:10:in `ensure in test': unrelated error (RuntimeError)
	from /tmp/test.rb:10:in `test'
	from /tmp/test.rb:14:in `<main>'
/tmp/test.rb:5:in `rescue in test': had an error (RuntimeError)
	from /tmp/test.rb:2:in `test'
	from /tmp/test.rb:14:in `<main>'
/tmp/test.rb:3:in `test': original error (RuntimeError)
	from /tmp/test.rb:14:in `<main>'

and this refactors to:

def test
  begin
    raise "original error"
  rescue
    raise "had an error"
  end

  begin
    foo = true
  ensure
    unless foo
      raise "unrelated error"
    end
  end
end

test

/tmp/test.rb:5:in `rescue in test': had an error (RuntimeError)
	from /tmp/test.rb:2:in `test'
	from /tmp/test.rb:16:in `<main>'
/tmp/test.rb:3:in `test': original error (RuntimeError)
	from /tmp/test.rb:16:in `<main>'

It prevents the ensure block coming into context unless the condition it is protecting against can exist, preventing a red herring during error diagnosis for the user.