Ilya Maximets <[email protected]> writes:

> On 28.01.2019 22:21, Ian Stokes wrote:
>> On 1/22/2019 1:22 PM, Ilya Maximets wrote:
>>> No need to implement dynamic vector to store arguments.
>>> 'svec' perfectly covers all the needed functionality.
>>>
>> 
>> Thanks for this Ilya, testing this this the last few days and seems
>> in working order, I see Aaron has also acked this, I just have one
>> comment below:
>> 
>>> Signed-off-by: Ilya Maximets <[email protected]>
>>> ---
>>>   lib/dpdk.c | 217 ++++++++++++++++-------------------------------------
>>>   1 file changed, 66 insertions(+), 151 deletions(-)
>>>
>>> diff --git a/lib/dpdk.c b/lib/dpdk.c
>>> index 0ee3e19c6..d70884aad 100644
>>> --- a/lib/dpdk.c
>>> +++ b/lib/dpdk.c
>>> @@ -38,6 +38,7 @@
>>>   #include "openvswitch/vlog.h"
>>>   #include "ovs-numa.h"
>>>   #include "smap.h"
>>> +#include "svec.h"
>>>   #include "vswitch-idl.h"
>>>     VLOG_DEFINE_THIS_MODULE(dpdk);
>>> @@ -75,65 +76,23 @@ process_vhost_flags(char *flag, const char
>>> *default_val, int size,
>>>       return changed;
>>>   }
>>>   -static char **
>>> -grow_argv(char ***argv, size_t cur_siz, size_t grow_by)
>>> -{
>>> -    return xrealloc(*argv, sizeof(char *) * (cur_siz + grow_by));
>>> -}
>>> -
>>> -static void
>>> -dpdk_option_extend(char ***argv, int argc, const char *option,
>>> -                   const char *value)
>>> -{
>>> -    char **newargv = grow_argv(argv, argc, 2);
>>> -    *argv = newargv;
>>> -    newargv[argc] = xstrdup(option);
>>> -    newargv[argc+1] = xstrdup(value);
>>> -}
>>> -
>>> -static char **
>>> -move_argv(char ***argv, size_t cur_size, char **src_argv, size_t src_argc)
>>> -{
>>> -    char **newargv = grow_argv(argv, cur_size, src_argc);
>>> -    while (src_argc--) {
>>> -        newargv[cur_size+src_argc] = src_argv[src_argc];
>>> -        src_argv[src_argc] = NULL;
>>> -    }
>>> -    return newargv;
>>> -}
>>> -
>>> -static int
>>> -extra_dpdk_args(const char *ovs_extra_config, char ***argv, int argc)
>>> -{
>>> -    int ret = argc;
>>> -    char *release_tok = xstrdup(ovs_extra_config);
>>> -    char *tok, *endptr = NULL;
>>> -
>>> -    for (tok = strtok_r(release_tok, " ", &endptr); tok != NULL;
>>> -         tok = strtok_r(NULL, " ", &endptr)) {
>>> -        char **newarg = grow_argv(argv, ret, 1);
>>> -        *argv = newarg;
>>> -        newarg[ret++] = xstrdup(tok);
>>> -    }
>>> -    free(release_tok);
>>> -    return ret;
>>> -}
>>> -
>>>   static bool
>>> -argv_contains(char **argv_haystack, const size_t argc_haystack,
>>> -              const char *needle)
>>> +args_contains(const struct svec *args, const char *needle)
>> 
>> I think 'needle' only makes sense as an argument name when haystack
>> was also passed :), but without haystack it seems arbitrary, I
>> suggest it be changed to something like like value.
>
> Sure.
> I'd like to also replace both "dpdk_extras" with "dpdk-extra" and fix the too 
> deep
> indentation near "auto_determine = false;". I'll send the v2 with these 
> changes.
> Hope I could keep the Ack with these changes.

If it's just whitespace and the log correction, then please do. :)

> I'll also add the "--socket-limit" patch to the set as it will need some 
> trivial rebase.
>
> Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to