Re: [PATCH] external_acl_type format cleanup pt 1

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.

The default case in that switch is missing so that we can easily
identify codes which are not producing display output - like these ones.
The XXX is because we need to make them have output there in the
Format:: API before we can use it more for external_acl_type.

>
> If this patch requires admins to rename any %macros, please provide a
> renaming table in release notes or at least in the proposed commit message.
>
> If this patch removes any existing functionality (from the admin point
> of view), please say so explicitly.
>

This patch does not affect existing configs.

The external_acl_type parser is extended to accept both the old and new
values for its existing format field tokens. It does not accept any
actually new tokens or new token meanings.

The new tokens added to logformat directive are not expected to be used
by any existing config files.

Amos
Received on Tue May 06 2014 - 16:59:56 MDT