| Message ID | 5a12f37d427abd020bf839ad68d560811453dada.1749133911.git.echaudro@redhat.com |
|---|---|
| State | Accepted |
| Commit | 2c634482f2c3ba3c9edd6f5c95dc7e7a67a50abf |
| Delegated to: | aaron conole |
| Headers | show |
| Series | Various Coverity fixes. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On Thu, Jun 5, 2025 at 10:52 AM Eelco Chaudron via dev <ovs-dev@openvswitch.org> wrote: > > The Raft codebase includes calls to ovsdb_log_write_and_free() that > are incorrectly wrapped in ignore(). This causes potential error > resources to be leaked. > > These calls should be wrapped in ovsdb_error_destroy() instead, to > ensure that any returned error objects are properly freed and do not > result in memory leaks. > > Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered databases.") > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> Acked-by: Mike Pattrick <mkp@redhat.com>
Eelco Chaudron via dev <ovs-dev@openvswitch.org> writes: > The Raft codebase includes calls to ovsdb_log_write_and_free() that > are incorrectly wrapped in ignore(). This causes potential error > resources to be leaked. > > These calls should be wrapped in ovsdb_error_destroy() instead, to > ensure that any returned error objects are properly freed and do not > result in memory leaks. > > Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for > clustered databases.") > Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > --- Amusingly, the ovsdb_log_write_and_free is marked as OVS_WARN_UNUSED_RESULT to prevent ignoring the result. Acked-by: Aaron Conole <aconole@redhat.com> > ovsdb/raft.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/ovsdb/raft.c b/ovsdb/raft.c > index 78ae39e84..9694c94e6 100644 > --- a/ovsdb/raft.c > +++ b/ovsdb/raft.c > @@ -1261,7 +1261,8 @@ raft_record_note(struct raft *raft, const char *note, > .comment = comment, > .note = CONST_CAST(char *, note), > }; > - ignore(ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); > + ovsdb_error_destroy( > + ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); > > free(comment); > } > @@ -2996,7 +2997,8 @@ raft_become_leader(struct raft *raft) > .term = raft->term, > .sid = raft->sid, > }; > - ignore(ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); > + ovsdb_error_destroy( > + ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); > > /* Initiate a no-op commit. Otherwise we might never find out what's in > * the log. See section 6.4 item 1: > @@ -3224,7 +3226,8 @@ raft_update_commit_index(struct raft *raft, uint64_t new_commit_index) > .type = RAFT_REC_COMMIT_INDEX, > .commit_index = raft->commit_index, > }; > - ignore(ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); > + ovsdb_error_destroy( > + ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); > return true; > } > > @@ -3412,7 +3415,8 @@ raft_update_leader(struct raft *raft, const struct uuid *sid) > .term = raft->term, > .sid = *sid, > }; > - ignore(ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); > + ovsdb_error_destroy > + (ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); > } > if (raft->role == RAFT_CANDIDATE) { > /* Section 3.4: While waiting for votes, a candidate may
On 5 Jun 2025, at 19:58, Mike Pattrick wrote: > On Thu, Jun 5, 2025 at 10:52 AM Eelco Chaudron via dev > <ovs-dev@openvswitch.org> wrote: >> >> The Raft codebase includes calls to ovsdb_log_write_and_free() that >> are incorrectly wrapped in ignore(). This causes potential error >> resources to be leaked. >> >> These calls should be wrapped in ovsdb_error_destroy() instead, to >> ensure that any returned error objects are properly freed and do not >> result in memory leaks. >> >> Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered databases.") >> Signed-off-by: Eelco Chaudron <echaudro@redhat.com> > > Acked-by: Mike Pattrick <mkp@redhat.com> Thanks for the review Aaron and Mike! Applied to main. //Eelco
diff --git a/ovsdb/raft.c b/ovsdb/raft.c index 78ae39e84..9694c94e6 100644 --- a/ovsdb/raft.c +++ b/ovsdb/raft.c @@ -1261,7 +1261,8 @@ raft_record_note(struct raft *raft, const char *note, .comment = comment, .note = CONST_CAST(char *, note), }; - ignore(ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); + ovsdb_error_destroy( + ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); free(comment); } @@ -2996,7 +2997,8 @@ raft_become_leader(struct raft *raft) .term = raft->term, .sid = raft->sid, }; - ignore(ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); + ovsdb_error_destroy( + ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); /* Initiate a no-op commit. Otherwise we might never find out what's in * the log. See section 6.4 item 1: @@ -3224,7 +3226,8 @@ raft_update_commit_index(struct raft *raft, uint64_t new_commit_index) .type = RAFT_REC_COMMIT_INDEX, .commit_index = raft->commit_index, }; - ignore(ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); + ovsdb_error_destroy( + ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); return true; } @@ -3412,7 +3415,8 @@ raft_update_leader(struct raft *raft, const struct uuid *sid) .term = raft->term, .sid = *sid, }; - ignore(ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); + ovsdb_error_destroy + (ovsdb_log_write_and_free(raft->log, raft_record_to_json(&r))); } if (raft->role == RAFT_CANDIDATE) { /* Section 3.4: While waiting for votes, a candidate may
The Raft codebase includes calls to ovsdb_log_write_and_free() that are incorrectly wrapped in ignore(). This causes potential error resources to be leaked. These calls should be wrapped in ovsdb_error_destroy() instead, to ensure that any returned error objects are properly freed and do not result in memory leaks. Fixes: 1b1d2e6daa56 ("ovsdb: Introduce experimental support for clustered databases.") Signed-off-by: Eelco Chaudron <echaudro@redhat.com> --- ovsdb/raft.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)