[tor-bugs] #2046 [Tor Client]: Port Tor code for starting a background process to Windows

Tor Bug Tracker & Wiki torproject-admin at torproject.org
Thu Aug 25 19:00:18 UTC 2011

#2046: Port Tor code for starting a background process to Windows
 Reporter:  sjmurdoch   |          Owner:  sjmurdoch         
     Type:  task        |         Status:  needs_review      
 Priority:  major       |      Milestone:  Tor: 0.2.3.x-final
Component:  Tor Client  |        Version:                    
 Keywords:              |         Parent:  #1983             
   Points:              |   Actualpoints:                    

Comment(by nickm):

 From IRC:


 20:19 < nickm> sjmurdoch: Initial request, pending more detailed review:
 Can we
                split the C string processing stuff out of functions like
                log_from_handle, so that we can do a unit test for it?
 20:21 < nickm> sjmurdoch: Also, elsewhere thoughout the code, we return
                on the heap, not by value. Any reason to make an exception
 20:21 < sjmurdoch> nickm: Yes, that sounds like a good idea
 20:21 < nickm> (If there's other string processing goop in the spawn code,
                logic applies)
 20:21 < sjmurdoch> nickm: Not particularly
 20:22 < sjmurdoch> nickm: You mean pass in a pointer which the function
 20:22 < sjmurdoch> or the function allocates the struct and returns a
 20:22 < nickm> Either one would be fine IMO.

 20:22 < sjmurdoch> nickm: OK
 20:24 < nickm> If the caller will generally just have it on the stack,
                a pointer is fine.
 20:24 < nickm> If the caller will generally want to have the struct stick
                around for a while, having the function allocate and return
                is better.  You'd probably want to add a process_handle_t
                function somewhere as well.
 20:25 < nickm> sjmurdoch: (Also, is it okay if I copy-and-paste all of
                onto the 2046 ticket when we're done?)
 20:25 < nickm> err, a process_handle_free function
 20:26 < sjmurdoch> Currently the process_handle_t instance for the helper
 is a
                    static variable in tor_check_port_forwarding
 20:26 < sjmurdoch> So I could pass in a pointer from
                    to tor_spawn_background?
 20:27 < nickm> fine by me
 20:27 < sjmurdoch> nickm: And yes, feel free to paste into 2046
 20:28 < nickm> ok
 20:29 < nickm> so, what happens if some argv entry contains a space?
 20:29 < sjmurdoch> You're looking at the join code?
 20:30 < nickm> (Yeah.) That'll happen for sure if we ever need to run a
                that lives under C:\Program Files\ .
 20:30 < sjmurdoch> For Windows, it would have to be put in quotes
 20:30 < nickm> sjmurdoch: I think we should have a separate, unit-tested
                function, that formats a command line properly for windows.
 20:31 < sjmurdoch> OK, I can refactor that out
 20:31 < nickm> great
 20:32 < nickm> also it should handle internal quotes; there have been way
                many shell-based snafus in the history of programming for
 us to
                be blase about anything other than stricly correct command
 20:32 < sjmurdoch> I don't fully understand how command lines are parsed
                    Windows. I do know that it is the process which is
                    that does it, but is it their libc?

 20:35 < nickm> I'm afraid I don't fully know.  But look at Python's
                subprocess.list2cmdline and at
 20:35 < nickm> I think it's actually done in the equivalent of crt.o
 20:36 < sjmurdoch> Thanks, I'll take a look
 20:41 < nickm> For tor_get_exit_code: Let's return a define or an enum
                than a magic tristate.
 20:42 < sjmurdoch> Sounds sensible
 20:42 < nickm> nitpick: Should "log_from_handle" be called "log_to_handle"
 20:43 < sjmurdoch> I think log_from_handle because it reads from a handle
                    logs the output
 20:43 < nickm> Ah!
 20:44 < nickm> makes sense
 20:44 < nickm> In tor_read_all_from_handle, I'd be more comfortable if it
                an tor_assert(byte_count + numread <= count);

 20:47 < nickm> sjmurdoch: In tor_log_from_handle: Do we care about strings
                embedded NULs?
 20:48 < sjmurdoch> nickm: Hmm, possibly
 20:48 < sjmurdoch> nickm: tor-fw-helper won't generate them but other
 20:49 < nickm> probably something to note; not necessarily something to
                right now.
 20:49 < sjmurdoch> OK
 20:49 < sjmurdoch> Related to this, I've assumed I'm working with byte
                    everywhere. Is Windows Unicode support going to bite
 20:50 < sjmurdoch> (another bit I don't understand well).
 20:50 < nickm> Not sure.  I think we're probably okay here.
 20:51 < nickm> Hm.  log_from_handle will do badly if the subprocess has
                than 255 bytes to say.
 20:52 < sjmurdoch> I think it will output partial lines in that case
 20:52 < nickm> yeah.
 20:52 < nickm> It might do bogus newlines if the \r\n is split
 20:53 < sjmurdoch> Hmm, yes
 20:54 < sjmurdoch> I'm not sure what to do here. Code to handle partial
                    will be tricky. On *nix I got away with using fgets but
                    fgets on Windows can't ready from a handle
 20:55 < nickm> I'd say for now, document it as a known limitation
 20:55 < nickm> For later, you'd need to leave any partial line in a buffer
                associated with the handle
 20:55 < nickm> and make the whole thing more stateful
 20:56 < sjmurdoch> OK
 20:57 < sjmurdoch> nickm: There is already a ticket related to this:
 20:57 < nickm> sjmurdoch: .... and with that, I'm done with the initial
                I think.

Ticket URL: <https://trac.torproject.org/projects/tor/ticket/2046#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online

More information about the tor-bugs mailing list