Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid nil conditions #106

Open
delaneyj opened this issue Jul 1, 2024 · 1 comment
Open

Invalid nil conditions #106

delaneyj opened this issue Jul 1, 2024 · 1 comment
Labels
code health Maintainability concern

Comments

@delaneyj
Copy link

delaneyj commented Jul 1, 2024

In savepoint.go

[{
	"resource": "zombiezen.com/go/[email protected]/sqlitex/savepoint.go",
	"owner": "_generated_diagnostic_collection_name_#2",
	"code": {
		"value": "cond",
		"target": {
			"$mid": 1,
			"path": "/golang.org/x/tools/go/analysis/passes/nilness",
			"scheme": "https",
			"authority": "pkg.go.dev",
			"fragment": "cond"
		}
	},
	"severity": 4,
	"message": "impossible condition: nil != nil",
	"source": "nilness",
	"startLineNumber": 95,
	"startColumn": 17,
	"endLineNumber": 95,
	"endColumn": 17
},{
	"resource": "zombiezen.com/go/[email protected]/sqlitex/savepoint.go",
	"owner": "_generated_diagnostic_collection_name_#2",
	"code": {
		"value": "cond",
		"target": {
			"$mid": 1,
			"path": "/golang.org/x/tools/go/analysis/passes/nilness",
			"scheme": "https",
			"authority": "pkg.go.dev",
			"fragment": "cond"
		}
	},
	"severity": 4,
	"message": "impossible condition: nil != nil",
	"source": "nilness",
	"startLineNumber": 209,
	"startColumn": 17,
	"endLineNumber": 209,
	"endColumn": 17
}]
@zombiezen zombiezen added the code health Maintainability concern label Jul 6, 2024
@zombiezen
Copy link
Owner

Interesting. So the relevant code is here on line 95:

if *errp == nil && recoverP == nil {
// Success path. Release the savepoint successfully.
*errp = Execute(conn, fmt.Sprintf("RELEASE %q;", name), nil)
if *errp == nil {
return
}
// Possible interrupt. Fall through to the error path.
if conn.AutocommitEnabled() {
// There is nothing to rollback.
if recoverP != nil {
panic(recoverP)
}
return
}
}

(The second report on line 209 is a copy & paste of the same.)

This is also present in the crawshaw repository, so this isn't a regression. In examining it, I think this is just dead code, not a defect. However, it would be nice to clean up the surrounding code for easier reading. Thanks for the report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health Maintainability concern
Projects
None yet
Development

No branches or pull requests

2 participants