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.
--
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
