nft parser and names for constants (was [PATCH v2] parser: add kludges for "param-problem" and "redirect")

[Alexander Holler <holler@ahsoftware.de>]

Am 06.04.2015 um 13:25 schrieb Patrick McHardy:
> On 06.04, Alexander Holler wrote:
>> Am 06.04.2015 um 11:01 schrieb Alexander Holler:
>>> Am 06.04.2015 um 10:44 schrieb Alexander Holler:
>>>> Am 06.04.2015 um 03:51 schrieb Patrick McHardy:
>>>>> On 05.04, Alexander Holler wrote:
>>>>>> Am 05.04.2015 um 14:11 schrieb Patrick McHardy:
>>>>>>> On 05.04, Patrick McHardy wrote:
>>>>>>
>>>>>>>> Basically this involves splitting the expression types into lhs
>>>>>>>> (non-const)
>>>>>>>> and rhs (const) parts. Keywords on the RHS side can be caught
>>>>>>>> using an
>>>>>>>> error statement and deferred to resolution during runtime.
>>>>>>
>>>>>> Sounds like trial and error. ;)
>>>>>
>>>>> The approach is, the patch isn't, it changes the grammar to have
>>>>> these kinds of errors in a defined state. The patch I sent
>>>>> however is, but I'm quite sure i understand the implications.
>>>>
>>>> Just to mention it, there is still the possibility to define and use
>>>> keywords for all the icmp type names.
>>>
>>> Preferable with names as used in icmp.h and icmpv6.h. As these are
>>> defines in C-headers, there is very high probability that these names
>>> are unique, even across a large number of different sets of type or
>>> other names.
>>
>> That would also remove the need to look up what name nft uses if would be
>> clear that the names are the same as defined in c-headers.
>>
>> E.g. the ICMPv6 parameter-problem is good example. In the linux headers it
>> is called ICMPV6_PARAMPROB, nft named it param-problem and in documentations
>> it is often named as parameter-problem.
>>
>> So if nft would use icmpv6_paramprob, the documentation could just refer to
>> the c-headers.
>
> No, an icmpv6_ prefix is redundant and I don't see any benefit in doing
> that. "Documentations", whatever that is, don't matter, what matters is
> our documentation. And whether we point people to that or some header
> file really doesn't matter, except that we don't expect people to read
> header files to use nft. Its a tool for admins, not programmers.

Even some admins can code.


Because nft is just at 0.4 and not widely used (or usable), I decided 
it's worse to write another follow up in order to try to fix things 
before they can't be fixed anymore.


1. I don't think that a brutforce-approach like "if error try something 
else" is the right way to fix a problem in the parser.

2. "icmpv6_paramprob" (or even something like 
"icmpv6_parameter-problem") doesn't have something redundant. It's a 
name for constant and e.g. "icmp_redirect" is a much more descriptive 
name than just "redirect" because "icmp_redirect" describes the context 
too and thus the name is much more verbose and readable (besides that it 
would fix the problem the parser currently has).

3. I don't see why admins have to use another set of names for constants 
than developers. Inventing a new set of names for a list of constants 
for which there already exist a very widely used set of names just leads 
to more confusion. And if it's ok to invent new names, why does nft use 
"param-problem" and not "parameter-problem"? Of course, I would suggest 
to use the existing name icmp_parameterprob (like it's used in every 
c/c++-source).

Alexander Holler