sqlite: fix issue in ValidateDBConfig() #20810
Conversation
openshift-ci
bot
added
release-note-none
approved
If a transaction is started it must either be committed or rolled back. The function uses defer to call `tx.Rollback()` if there is an error returned. However it also called `tx.Commit()` and afterwards further errors can be returned which means it tries to roll back a already committed transaction which cannot work. This fix is to make sure tx.Commit() is the last call in that function. see containers#20731 [NO NEW TESTS NEEDED] Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Luap99
force-pushed
the
sqlite-validate
branch
from
48b4f0c
to
d7b970a
Compare
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: giuseppe, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -373,10 +373,6 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) { | |||
| return fmt.Errorf("retrieving DB config: %w", err) | |||
| } | |||
|
|
|||
| if err := tx.Commit(); err != nil { | |||
| return fmt.Errorf("committing database validation row: %w", err) | |||
| } | |||
There was a problem hiding this comment.
Maybe a stupid question, but in the interests of keeping the transaction as short as possible, might it make sense to instead keep this here, add tx = nil, and check for tx != nil in the defer?
There was a problem hiding this comment.
we can do that but the checks are just string compare so it is very fast anyway
There was a problem hiding this comment.
Thanks. LGTM as it is.
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
If a transaction is started it must either be committed or rolled back. The function uses defer to call
tx.Rollback()if there is an error returned. However it also calledtx.Commit()and afterwards further errors can be returned which means it tries to roll back a already committed transaction which cannot work.This fix is to make sure tx.Commit() is the last call in that function. see #20731
Does this PR introduce a user-facing change?