[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