DIRCACHE_MIN_MB_BANDWIDTH is actually used for RAM

We should change DIRCACHE_MIN_BANDWIDTH and DIRCACHE_MIN_MB_BANDWIDTH to end in *_MEM, as they are only ever used for memory, not bandwidth.

Introduced in commit 997f779 in tor-0.2.8.1-alpha.

changed milestone to %Tor: 0.3.0.x-final

added component::core tor/tor easy intro milestone::Tor: 0.3.0.x-final owner::dgoulet points::0.1 priority::medium resolution::fixed severity::normal status::closed type::defect typo version::tor 0.2.8.1-alpha labels

Trac:
tor_patch_typo.patch

Patch to rename DIRCACHE_MIN_BANDWIDTH and DIRCACHE_MIN_MB_BANDWIDTH to DIRCACHE_MIN_BANDWIDTH_MEM and DIRCACHE_MIN_MB_BANDWIDTH_MEM

Hi,

I have created a patch to fix this. Please tell me what you think about this.

Thank You, Neel Chauhan

Hi Neel, thanks for the patch.

Can you please rename them to: DIRCACHE_MIN_MEM and DIRCACHE_MIN_MB_MEM (There is no need to say "BANDWIDTH" at all, because it is confusing to say "BANDWIDTH" when talking about memory.) I am sorry I did not explain better in the ticket. Since these new names are shorter, there is no need to change the log messages to pass make check-spaces.

Also, would you like to do a changes file for this one? Changes files help us make the changelog at the end of the release. You can say "patch by Neel Chauhan" at the end if you want. Here's how we do a changes file: https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandards.md#n56 Or you can look at the one I made on your last ticket for an example.

For the changes file, this bug was introduced in tor-0.2.8.1-alpha.

I used git blame to find out this change was made in commit 997f779, and git describe --contains 997f779 to find out the release for this commit was tor-0.2.8.1-alpha.

Trac:
Status: new to needs_revision

Bug #20887 (moved) is another bug in this code.

Trac:
0001-rename-DIRCACHE_MIN_BANDWIDTH-and-DIRCACHE_MIN_MB_BA.patch

Updated patch to rename DIRCACHE_MIN_BANDWIDTH and DIRCACHE_MIN_MB_BANDWIDTH to DIRCACHE_MIN_MEM and DIRCACHE_MIN_MB_MEM

Trac:
Cc: N/A to neel@neelc.org

Trac:
Status: needs_revision to needs_review

Since this fix changes the message output when the memory is too low, the following unit test fails:

options/mem_dircache: [forking] 
  FAIL src/test/test_options.c:236: assert(r OP_EQ -1): 0 vs -1
  [mem_dircache FAILED]

The best fix for this is to fix #20887 (moved), then change the message expected by the unit test.

Trac:
Status: needs_review to needs_revision

Actually, the issue was that DIRCACHE_MIN_MEM was used instead of DIRCACHE_MIN_MB_MEM in the test. I've fixed it using the attached commit. Test passes now.

See branch bug20684_030_01

Trac:
Status: needs_revision to assigned
Owner: N/A to dgoulet

Trac:
Status: assigned to merge_ready

I think the convention here is (or should be) to use a suffix that denotes the units of the constant. So I've added an extra commit (e0e729d4b52ef4a14e63e194ff0a47a0aa0d0f99) and merged this.

(We don't need to add a "BYTES" suffix to everything that is already in bytes.)

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

closed

changed time estimate to 48m

mentioned in issue #20887 (moved)

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

mentioned in issue tpo/core/tor#20887 (closed)