Opened 3 weeks ago

Closed 13 days ago

#31854 closed defect (fixed)

In tests and log.c, stop using ~0 a log domain mask

Reported by: nickm Owned by: teor
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: 042-should easy fast-fix asn-merge
Cc: Actual Points: 0.2
Parent ID: #30901 Points: 0.2
Reviewer: nickm Sponsor:

Description

There are a few places in the tests where we use ~0 or ~0u to indicate a log domain mask that covers all domains. We also do this in log.c.

But back in #31080, we made the log_domain_mask_t into a 64-bit value, probably one defined by a macro like LD_ALL_DOMAINS.

Additionally, we should not use ~(uint64_t)0 for the definition of this value, since we don't want to include LD_NO_MOCK, LD_NOCB, and LD_NOFUNCNAME.

Found while looking at #31334; this should be done after #31334 is merged.

No backport needed, since we do not yet have any logging domains that use the high 32 bits of this type.

Child Tickets

Change History (6)

comment:1 Changed 3 weeks ago by teor

Owner: set to teor
Parent ID: #30901
Status: newassigned

I'll do this as part of #30901, because I'll be adding a "no control logs" domain. And possibly renaming NO_CB to DEFER_CB.

comment:2 Changed 3 weeks ago by teor

Actual Points: 0.2
Points: 0.2
Reviewer: nickm
Status: assignedneeds_review
Version: Tor: unspecified

See my PR:

It will have a minor conflict with #31334, I'll fix whichever one merges last.

comment:3 Changed 3 weeks ago by nickm

Keywords: asn-merge added
Status: needs_reviewmerge_ready

This looks okay to me.

comment:4 Changed 2 weeks ago by asn

Status: merge_readyneeds_revision

Yep, there is a non-trivial conflict I'm afraid. Moving this to needs_revision.

comment:5 Changed 2 weeks ago by teor

Status: needs_revisionmerge_ready

I rebased on master.

Three lines in the last commit in test_options.c had conflicts. I merged the changes from #31334 with these changes.

See my PR:

comment:6 Changed 13 days ago by asn

Resolution: fixed
Status: merge_readyclosed

Merged!

Note: See TracTickets for help on using tickets.