Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#5091 closed defect (fixed)

tor_strtok_r_impl incompatible with strtok_r

Reported by: emanchado Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: unittest tor-client
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

tor_strtok_r_impl doesn't behave strtok_r in certain cases, eg:

  • On empty string, it should return NULL directly (rather than an empty string).
  • When one of the separators is at the end of the input string, there should be no extra token at the end (ie. if you split "howdy!" on "!", there should be a single token "howdy", not "howdy" and "").
  • When the input string consists exclusively of separator characters, there should be no tokens.

Child Tickets

Attachments (1)

0001-Fix-tor_strtok_r_impl-and-test-cases-per-bug-5091.patch (3.7 KB) - added by nils 8 years ago.
Patch to fix

Download all attachments as: .zip

Change History (9)

comment:1 Changed 8 years ago by rransom

Component: - Select a componentTor Client
Priority: minornormal

Changed 8 years ago by nils

Patch to fix

comment:2 Changed 8 years ago by nils

Status: newneeds_review

comment:3 Changed 8 years ago by Sebastian

Milestone: Tor: 0.2.3.x-final

Could be argued to be a bug fix, I suppose. Haven't reviewed yet

comment:4 Changed 8 years ago by nickm

For my benefit: all of the differences between the standard strtok_r and ours could be summarized as: "strtok_r should never return an empty token."

"git am" and "git apply" can't put this onto master cleanly; I'll need to figure out what to apply it to before merging it forward, or just reconstruct it by hand.

comment:5 Changed 8 years ago by nickm

Okay I tweaked this patch a little to make it apply cleanly to master, to extract some common code into a function, and to replace snprintf with tor_snprintf.

Please have a quick look at branch "bug5091" on my public repo if you have a chance?

Nils, do you want to be credited with any name more verbose than "nils", or is "nils" what you prefer?

comment:6 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

okay, nobody objects, so I'm merging.

(This is currently only used for parsing bandwidth files and resolv.conf files, so even if I'm totally wrong here, the risk is low.)

comment:7 Changed 7 years ago by nickm

Keywords: tor-client added

comment:8 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.