Use internal reputation variables to verify memory allocation

The reputation code (rephist.c) has internal variables to keep track on the memory allocated by its objects. However, when the bandwidth arrays are freed, it forgets to decrease the memory allocation counter. This leads to discrepancy between the recorded and the actual amount of memory that is allocated when the bandwidth arrays are reinitialized.

changed milestone to %Tor: 0.2.8.x-final

added component::core tor/tor milestone::Tor: 0.2.8.x-final priority::low resolution::fixed severity::minor status::closed type::defect version::tor unspecified labels

Trac:
0001-Assert-that-memory-held-by-rephist-is-freed.patch

Trac:
Status: new to needs_review

This patch looks good - thanks for making sure we don't leak this memory.

Can you explain the removal of the cast from the line: lhist = (link_history_t*) digestmap_get(orhist->link_history_map, to_id);

Did you mean to include it in the patch?

Trac:
Status: needs_review to needs_revision
Milestone: N/A to Tor: 0.2.8.x-final

Replying to teor:

Can you explain the removal of the cast from the line: lhist = (link_history_t*) digestmap_get(orhist->link_history_map, to_id); As explained in the commit message, i found this unnecessary cast while working on the code. The digestmap returns a pointer to void so it automatically converts to whatever pointer type you assign it to already, no cast necessary. Did you mean to include it in the patch? I did mean to include it in the patch because it was such a small change, but i agree that it is a bit unrelated. Should i split the commit in two?

Replying to cypherpunks:

Replying to teor:

Can you explain the removal of the cast from the line: lhist = (link_history_t*) digestmap_get(orhist->link_history_map, to_id); As explained in the commit message, i found this unnecessary cast while working on the code. The digestmap returns a pointer to void so it automatically converts to whatever pointer type you assign it to already, no cast necessary.

Thanks for picking this up, it's the only location in the entire codebase that we cast the result of digestmap_get. (For those circumstances where casting is necessary, there are automatically generated functions that cast to a type.)

Did you mean to include it in the patch? I did mean to include it in the patch because it was such a small change, but i agree that it is a bit unrelated. Should i split the commit in two?

There's no need to split this commit. (In future, please keep one commit per topic. It's easier for reviewers to understand.)

Thanks for the patch, I am happy with it. Let's get it merged.

Trac:
Status: needs_revision to needs_review

+1 with teor here on the split but else this patch lgtm also!

lgtm too; merging.

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

closed

moved to tpo/core/tor#17753 (closed)