On Thu, May 18, 2017 at 4:29 PM, Oliver Ford <ojford@xxxxxxxxx> wrote:
> On Thu, May 18, 2017 at 4:20 PM, Florian Westphal <fw@xxxxxxxxx> wrote:
>> Oliver Ford <ojford@xxxxxxxxx> wrote:
>>> On Thu, May 18, 2017 at 3:42 PM, Florian Westphal <fw@xxxxxxxxx> wrote:
>>> > Oliver Ford <ojford@xxxxxxxxx> wrote:
>>> >> --- a/iptables/ip6tables-restore.c
>>> >> +++ b/iptables/ip6tables-restore.c
>>> >> @@ -165,14 +165,33 @@ static void add_param_to_argv(char *parsestart)
>>> >> param_buffer[param_len] = '\0';
>>> >>
>>> >> /* check if table name specified */
>>> >> - if (!strncmp(param_buffer, "-t", 2)
>>> >> - || !strncmp(param_buffer, "--table", 8)) {
>>> >> + if (param_buffer[0] == '-' && param_buffer[1] != '-'
>>> >> + && strchr(param_buffer, 't')) {
>>> >> xtables_error(PARAMETER_PROBLEM,
>>> >> - "The -t option (seen in line %u) cannot be "
>>> >> - "used in ip6tables-restore.\n", line);
>>> >> + "The -t option (seen in line %u) cannot be "
>>> >> + "used in ip6tables-restore.\n", line);
>>> >> + exit(1);
>>> >> + } else if (!strncmp(param_buffer, "--", 2)
>>> >> + && strchr(param_buffer, 't')) {
>>> >
>>> > Why this strchr() ?
>>> >
>>> > if (!strncmp(param_buffer, "--t", 3) &&
>>> > !strncmp(param_buffer, "--table", strlen(param_buffer))
>>> > err();
>>> >
>>> > should work.
>>> >
>>>
>>> "strncmp(param_buffer, "--t", 3)" wouldn't return 0 if you had an
>>> invalid parameter like "--ftabl".
>>
>> Yes, but that will just throw an error:
>> iptables-restore v1.6.0: unknown option "--ftab"
>>
>>> >> + /* If we begin with a '--' and have a 't', check
>>> >> + * that the parameter is in the list of valid options */
>>> >> + const char* t_options[] = {
>>> >
>>> > If this is needed, I'd suggest
>>> >
>>> > static const char * const t_options[] = {
>>> >
>>> >> + "delete", "insert", "list", "list-rules", "delete-chain",
>>> >> + "destination", "dst", "protocol", "in-interface", "match",
>>> >> + "out-interface", "wait", "wait-interval", "exact",
>>> >> + "fragments", "set-counters", "goto"};
>>> >> + int i, opt_len = ARRAY_SIZE(t_options);
>>> >> + for (i = 0; i < opt_len; i++) {
>>> >> + if (!strcmp(param_buffer + 2, t_options[i])) {
>>> >> + goto t_passed;
>>> >> + }
>>> >> + }
>>> >
>>> > If this t_options[] thing is really needed i'd try to stick this into
>>> > a helper function so we don't have to duplicate this in all 3
>>> > incarnations.
>>> >
>>> > Thanks for working on this.
>>>
>>> Can do, but the code already duplicates everything else across all 3
>>> incarnations so I thought it best to stick with the established
>>> pattern :-) If you'd prefer it extracted I'll send a new version.
>>
>> That pattern sucks :-)
>
> Ok, I'll make version three with these changes.
I've just sent version 3 with a much simplified check. Now it filters
a single '-' with a 't' anywhere, or a '--t'. That seems to do the
trick.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
