[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #17744 [Tor]: Add quotes when comparing strings in configure script



#17744: Add quotes when comparing strings in configure script
-------------------------+------------------------------------
 Reporter:  cypherpunks  |          Owner:  zerosion
     Type:  defect       |         Status:  needs_review
 Priority:  Medium       |      Milestone:  Tor: 0.2.8.x-final
Component:  Tor          |        Version:  Tor: 0.2.7.5
 Severity:  Trivial      |     Resolution:
 Keywords:  easy         |  Actual Points:
Parent ID:               |         Points:
  Sponsor:               |
-------------------------+------------------------------------

Comment (by teor):

 Replying to [comment:13 zerosion]:
 > Done.

 Code review:

 I think it's good to quote `$host` in the unlikely event this external
 info has spaces or special characters. Similarly, I would accept other
 patches that quoted external or user-specified input.

 I think it's ok to quote literal file paths that don't currently have
 spaces. This is consistent with quoting user-specified paths, in case they
 have spaces or special characters.

 I can't see the point of quoting or changing the quotes around internal
 variables or internal string literals. These are the majority of the
 changes in the patch, and I'm not sure if they gain us anything. They need
 to be weighed against the risk that a misplaced quote breaks the build on
 some obscure configuration.
 Was there a reason for these changes?

 And finally, if we're going to tidy up whitespace, let's consider that in
 another ticket. (I'm not sure if we will want to, see my comments below.)

 > I've seen a lot of syntax irregularities, like tabs mixed with spaces
 and different indentation along the file. Should we open a ticket to
 handle this, or it's fine?

 Let's open a ticket to document it, but set it to low priority.
 The issue with re-flowing the entire file is that it makes it hard to tell
 when the last non-whitespace change was made to each line.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/17744#comment:14>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs