On 6/01/2013 1:12 p.m., Tianyin Xu wrote:
Hi, Amos,
I addressed your audit results one by one (see my reply below each
audit entry). The updated patches is attached.
Two things I want to mention in the very beginning:
1. I checked the version according to your information, and made sure
it's the newest one (revision 12566). :-)
2. For the precedence level, you mentioned "m*d/u == "m*(d/u)". I
checked both C and C++ reference manual*,**.
* M. A. Ellis and B. Stroustrup, "The Annotated C++ Reference
Manual", Addison-Wesley Publishing, Sept. 1999.
** S. P. Harbison and G. L. Steele Jr., "C: A Reference Manual (3rd
Edition)", Prentice Hall, 1991.
Both of them claims that multiplicative operators (include *, /, %)
have the same precedence level, and group left-to-right. So I think it
makes sense to rewrite m*d/u*2 to (m*d/u)*2 but not to m*(d/u)*2. Let
me know if I misunderstood.
Hmm. Okay. That will do.
* at line 3475. port is being set by the strtol() call in the if()
condition. There is no need to parse the token twice.
I kept it.
* the debugs you are adding at line 3705 appears to already be present in my
copy of trunk with better text.
Hmm. I'm wondering if you did get trunk checkout properly now.
I checked the newest version and that one is not there.:-)
Ah found it was a patch I was testing at the time. Sorry.
It added "debugs(3, DBG_CRITICAL, "FATAL: Unknown http(s)_port option '"
<< token << "'");"
in src/wccp2.cc:
* the second chunk is parsing a port value. It should probably be using
xatos() and p defined as a uint16_t.
I kept for the better logging.
audit results round 2:
in src/Parsing.cc:
* xatos() debugs messages are missing spaces after the token output:
"'cannot be less
"'is larger than
* spelling ... s/ is similar as / is similar to /
Those are all cosmetic and can be changed on commit along with a
re-format to fix the whitespace.
So, unless there are any others with objections I'm happy to commit
this. +1.
Amos
