Re: [PATCH] Limit log field width

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