Re: [PATCH] external_acl_type format cleanup pt 1

On 7/05/2014 8:00 a.m., Alex Rousskov wrote:
> On 05/06/2014 10:59 AM, Amos Jeffries wrote:
>> On 7/05/2014 4:01 a.m., Alex Rousskov wrote:
>>> On 05/06/2014 08:37 AM, Amos Jeffries wrote:
>>>
>>>> Several of the external ACL format codes have been added to
>>>> Format::ByteCode_t without equivalent logformat TokenTableEntry's
>>>
>>>> @ Format::Format::assemble(...)
>>>> + // XXX: external_acl_type format codes are not yet output by this code
>>>> + case LFT_EXT_ACL_USER_CERT_RAW:
>>>> + case LFT_EXT_ACL_USER_CERTCHAIN_RAW:
>>>> + case LFT_EXT_ACL_USER_CERT:
>>>> + case LFT_EXT_ACL_USER_CA_CERT:
>>>> + case LFT_EXT_ACL_CLIENT_EUI48:
>>>> + case LFT_EXT_ACL_CLIENT_EUI64:
>>>> + case LFT_EXT_ACL_NAME:
>>>> + case LFT_EXT_ACL_DATA:
>>>
>>> I am not versed enough in Format terminology to quickly grok the above,
>>> so I have to ask: What does the above mean from the admin point of view?
>>> Does the proposed change completely removes support for some external
>>> ACL %macros? Or do we still support everything but require some %macro
>>> renaming for the old squid.confs to continue to work?
>>
>> It means they do not yet produce any output if used in config values
>> other than external_acl_type. There is also no TokenTableEntry using
>> them, so the logformat parser/lexer will also not parse any squid.conf
>> token into those codes.
>
> Sounds good. In summary, certain %codes remain specific to the
> external_acl_type, but the code is massaged to ease their future wider
> support.
>
>
>> - case _external_acl_format::EXT_ACL_UNKNOWN:
>>
>> - case _external_acl_format::EXT_ACL_END:
>> + default:
>> + // TODO: replace this function with Format::assemble()
>> + // For now die on unsupported logformat codes.
>> fatal("unknown external_acl format error");
>> break;
>> }
>
> If it is easy, please use fatalf() to report the offending %code name.
> That would help folks identifying the offending %code among the many
> suspects on a logformat line. This specific change does not warrant
> another round of reviews IMO.
>

We only have access to the numeric enum code at that point.
But done and applied.

Thank you
Amos
Received on Wed May 07 2014 - 10:35:20 MDT