From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Holler Subject: nft parser and names for constants (was [PATCH v2] parser: add kludges for "param-problem" and "redirect") Date: Thu, 09 Apr 2015 12:52:06 +0200 Message-ID: <552659D6.8090902@ahsoftware.de> References: <551FC211.6000907@ahsoftware.de> <1428145986-15421-1-git-send-email-holler@ahsoftware.de> <20150404115550.GA5832@salvia> <20150405113214.GA23433@acer.localdomain> <20150405121104.GD23433@acer.localdomain> <552187FB.60904@ahsoftware.de> <20150406015128.GA20515@acer.localdomain> <55224776.4040108@ahsoftware.de> <55224B70.1070309@ahsoftware.de> <55224E82.3010206@ahsoftware.de> <20150406112543.GB24191@acer.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: Pablo Neira Ayuso , netfilter-devel@vger.kernel.org, Arturo Borrero Gonzalez , Eric Leblond To: Patrick McHardy Return-path: Received: from h1446028.stratoserver.net ([85.214.92.142]:35687 "EHLO mail.ahsoftware.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933205AbbDIKwS (ORCPT ); Thu, 9 Apr 2015 06:52:18 -0400 Received: from wandq.ahsoftware (p4FC379E9.dip0.t-ipconnect.de [79.195.121.233]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.ahsoftware.de (Postfix) with ESMTPSA id E2C1D2C9C1BB for ; Thu, 9 Apr 2015 12:52:14 +0200 (CEST) In-Reply-To: <20150406112543.GB24191@acer.localdomain> Sender: netfilter-devel-owner@vger.kernel.org List-ID: 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