Opened 22 months ago

Closed 6 months ago

#20887 closed defect (fixed)

DIRCACHE_MIN_MEM_MB does not stringify on FreeBSD, we should use %d instead

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.3.4.x-final
Component: Core Tor/Tor Version: Tor: 0.2.8.1-alpha
Severity: Normal Keywords: easy, refactor, logging, tor-relay
Cc: neel@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by arma)

A FreeBSD relay operator reports the message:

[WARN] Being a directory cache (default) with less than DIRCACHE_MIN_MB_BANDWIDTH MB of memory is not recommended and may consume most of the available resources, consider disabling this functionality by setting the DirCache option to 0

So we should print the macro value the same way we do every other value, using "%d" in the string, and passing it as an argument.

(This macro changed name in #20684.)

Child Tickets

Attachments (2)

b20887-p1.patch (2.2 KB) - added by neel 6 months ago.
[PATCH] Change to tor_asprintf for DIRCACHE_MIN_MB_BANDWIDTH in have_enough_mem_for_dircache() (Revision 1)
b20887-p2.patch (2.2 KB) - added by neel 6 months ago.
[PATCH] Change to tor_asprintf for DIRCACHE_MIN_MB_BANDWIDTH in have_enough_mem_for_dircache() (Revision 1)

Download all attachments as: .zip

Change History (14)

comment:1 Changed 22 months ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:2 Changed 16 months ago by arma

Description: modified (diff)
Summary: DIRCACHE_MIN_MB_BANDWIDTH does not stringify on FreeBSD, we should use %d insteadDIRCACHE_MIN_MEM_MB does not stringify on FreeBSD, we should use %d instead

comment:3 Changed 16 months ago by arma

Agreed. Also, since we'll be using an arg, we want to use tor_asprintf() rather than tor_strdup().

This one's easy! Any new developers want to grab it?

comment:4 Changed 16 months ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:5 Changed 15 months ago by nickm

Keywords: logging tor-relay added; intro removed

comment:6 Changed 6 months ago by neel

Cc: neel@… added

Changed 6 months ago by neel

Attachment: b20887-p1.patch added

[PATCH] Change to tor_asprintf for DIRCACHE_MIN_MB_BANDWIDTH in have_enough_mem_for_dircache() (Revision 1)

comment:7 Changed 6 months ago by neel

I have a patch under the filename b20887-p1.patch​.

comment:8 Changed 6 months ago by teor

Status: newmerge_ready

Looks fine to me

comment:9 Changed 6 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.4.x-final
Status: merge_readyneeds_revision

There's a problem with this patch -- it removes the blank spaces before the quotation marks at the end of some of the lines. Without those blank spaces, there will not be appropriate spacing between words in the log message.

For example, this code

     tor_asprintf(msg, "Being a directory cache (default) with less than"
 		       "%d MB of memory is not recommended and may consume"
 		       "most of the available resources, consider disabling "
 		       "this functionality by setting the DirCache option "
 		       "to 0.", DIRCACHE_MIN_MEM_MB);

will produce a log message beginning with:

Being a directory cache (default) with less than300 MB of memory is not recommended and may consumemost of the available resources...

Note the "than300" and "consumemost" problem.

Also, while we're changing this, let's replace the comma before "consider" with a period: those are two separate sentences.

Changed 6 months ago by neel

Attachment: b20887-p2.patch added

[PATCH] Change to tor_asprintf for DIRCACHE_MIN_MB_BANDWIDTH in have_enough_mem_for_dircache() (Revision 1)

comment:10 in reply to:  9 Changed 6 months ago by neel

Replying to nickm:

There's a problem with this patch -- it removes the blank spaces before the quotation marks at the end of some of the lines. Without those blank spaces, there will not be appropriate spacing between words in the log message.

For example, this code

     tor_asprintf(msg, "Being a directory cache (default) with less than"
 		       "%d MB of memory is not recommended and may consume"
 		       "most of the available resources, consider disabling "
 		       "this functionality by setting the DirCache option "
 		       "to 0.", DIRCACHE_MIN_MEM_MB);

will produce a log message beginning with:

Being a directory cache (default) with less than300 MB of memory is not recommended and may consumemost of the available resources...

Note the "than300" and "consumemost" problem.

Also, while we're changing this, let's replace the comma before "consider" with a period: those are two separate sentences.

I made the changes in the patch b20887-p2.patch.

comment:11 Changed 6 months ago by nickm

lgtm; thanks for the fast turnaround! Merging to master.

comment:12 Changed 6 months ago by nickm

Resolution: fixed
Status: needs_revisionclosed

merged to master as bc5f79b95c5f809635bc84dbdd3010350bfbc269 with a tweak to the changes file so make check would pass.

Note: See TracTickets for help on using tickets.