add len argument to consensus parsing functions
Trac:
Username: cyberpunks
Activity
changed milestone to %Tor: 0.4.0.x-final
Created the branch parselen1 at https://gitgud.io/onionk/tor.git
Trac:
Username: cyberpunksThanks for this branch. It looks good to me, but I'd like to make sure that CI passes, and that someone who is more familiar with this code also does a review.
Please don't assign tickets to releases, we'll do that based on our roadmap for the release. (This task is on our roadmap as sponsored work, so we will prioritise reviewing it before the feature freeze on Friday.)
Can you explain the fix in 5d27c458 in the commit message and changes file? Each fix and feature also gets its own ticket number. We usually open a child ticket for each unrelated fix in a branch.
Or is this change just refactoring? For refactoring, the changes file should have the heading: Code simplification and refactoring https://github.com/torproject/tor/blob/master/scripts/maint/lintChanges.py#L22
Trac:
Status: new to needs_review
Sponsor: N/A to Sponsor8
Milestone: Tor: 0.3.6.x-final to Tor: 0.3.5.x-finalThe CI for this branch is here:
-
Replying to teor:
Or is this change just refactoring? For refactoring, the changes file should have the heading: Code simplification and refactoring https://github.com/torproject/tor/blob/master/scripts/maint/lintChanges.py#L22
Yeah, that was just simplifying, no behavior change replacing strncmp with strcmpstart. #27630 (moved).
So two changes files, both labeled as Code simplification and refactoring?
Trac:
Username: cyberpunks Replying to cyberpunks:
Replying to teor:
Or is this change just refactoring? For refactoring, the changes file should have the heading: Code simplification and refactoring https://github.com/torproject/tor/blob/master/scripts/maint/lintChanges.py#L22
Yeah, that was just simplifying, no behavior change replacing strncmp with strcmpstart. #27630 (moved).
So two changes files, both labeled as Code simplification and refactoring?
"use strcmpstart() in rend_parse_v2_service_descriptor" is definitely "Code simplification and refactoring".
The rest of the branch makes Tor use less RAM, so it can be: "Minor feature (RAM usage)".
Trac:
Type: defect to enhancement-
Replying to teor:
The rest of the branch makes Tor use less RAM
How does it do that? Well, there's removal of an allocation in the fuzz test. But no reduction in any actually running Tor client. This branch is just prerequisite refactoring.
There is another branch that does the actual switch to using mmap, but it's based on top of #27247 (moved) due to conflicts, so waiting on that to be merged.
Trac:
Username: cyberpunks Replying to cyberpunks:
Replying to teor:
The rest of the branch makes Tor use less RAM
How does it do that? Well, there's removal of an allocation in the fuzz test. But no reduction in any actually running Tor client. This branch is just prerequisite refactoring.
There is another branch that does the actual switch to using mmap, but it's based on top of #27247 (moved) due to conflicts, so waiting on that to be merged.
Oh, right. I looked at all the commits, including the mmap ones that are in parselen1-on-ticket27247.
So for parselen1, it's just refactoring. For the mmap branch, it's an enhancement.
why is this needed in relation to the patches in #27244 (moved) ? This seems like a lot of clean up patches in related code?
-
Replying to ahf:
why is this needed in relation to the patches in #27244 (moved) ? This seems like a lot of clean up patches in related code?
It's the same work. That's why it's a child ticket. All these functions assumed strings are NUL-terminated, which they aren't if passed something mmap'd, so they need explicit length added.
The branch parselen1-on-ticket27247 finishes the job of #27244 (moved). All this stuff needs to be rebased now though.
Trac:
Username: cyberpunks moved to tpo/core/tor#27629 (closed)
