Alex Rousskov wrote:
On 01/21/2010 05:19 AM, Amos Jeffries wrote:
Alex Rousskov wrote:
Hello,
<snip>
TODO: The name comes from the printf(3) "precision" format part. It may
be a good idea to rename our "precision" into max_width or similar,
especially if we do not support floating point precision logging.
" [width[.maximum]] " gets my vote. seems clear and concise.
The maximum width field does not require the width field. Should I
change to [width][.maximum] or [width_min].[width_max]? These are just
labels in the .pre -- there is no effect on the code.
Yes definitely needs to be clarified there. The BNF fooled me even with
your example below it.
I'll agree with your latter naming with that independence.
TODO: Old code used chars to store user-configured field width and
precision. That does not work for URLs, headers, and other entries
longer than 256 characters. This patch changes the storage type to int.
The code should probably be polished further to remove unsigned->signed
conversions.
---------------------
Please review.
+1. If the TODO can be removed.
You are talking about the "unsigned->signed" TODO? I am happy to make
those changes, but they will make the patch much bigger, there is higher
risk of compilation warnings, and they are not really about the feature
in question. Should I still do it? Perhaps as a separate commit?
Up to you.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE7 or 3.0.STABLE21
Current Beta Squid 3.1.0.15
