On 1/15/24 16:58, Simon Horman wrote:
> On Fri, Jan 12, 2024 at 02:54:03PM +0100, Ilya Maximets wrote:
>> On 1/12/24 11:44, Simon Horman wrote:
>>> On Tue, Jan 09, 2024 at 11:49:04PM +0100, Ilya Maximets wrote:
>>>> Small refactoring so we can re-use this function in later commits.
>>>>
>>>> Signed-off-by: Ilya Maximets <[email protected]>
>>>> ---
>>>>  ovsdb/ovsdb-server.c | 45 +++++++++++++++++++++++++++++++-------------
>>>>  1 file changed, 32 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
>>>> index 7f65cadfe..7e95b3813 100644
>>>> --- a/ovsdb/ovsdb-server.c
>>>> +++ b/ovsdb/ovsdb-server.c
>>>> @@ -316,6 +316,34 @@ main_loop(struct server_config *config,
>>>>      free(remotes_error);
>>>>  }
>>>>  
>>>> +/* Parsing the relay in format 'relay:DB_NAME:<list of remotes>'.
>>>> + * On success, returns 'true', 'name' is set to DB_NAME, 'remotes' to
>>>> + * '<list of remotes>'.  Caller is responsible of freeing 'name' and
>>>> + * 'remotes'.  On failure, returns 'false'.  */
>>>> +static bool
>>>> +parse_relay_args(const char *arg, char **name, char **remote)
>>>> +{
>>>> +    const char *relay_prefix = "relay:";
>>>> +    const int relay_prefix_len = strlen(relay_prefix);
>>>> +    bool is_relay;
>>>> +
>>>> +    is_relay = !strncmp(arg, relay_prefix, relay_prefix_len);
>>>> +    if (!is_relay) {
>>>> +        return false;
>>>> +    }
>>>> +
>>>> +    *remote = strchr(arg + relay_prefix_len, ':');
>>>> +
>>>> +    if (!*remote || (*remote)[0] == '\0') {
>>>> +        *remote = NULL;
>>>> +        return false;
>>>
>>> Hi Ilya,
>>>
>>> Prior to this patch this condition would cause open_db()
>>> to return an "invalid syntax" error message. Now it
>>> will cause the (!is_relay) branch to be taken in open_db().
>>>
>>> Is this intended?
>>
>> It's a side effect of extracting the function.  But I'm not sure
>> if it is a big deal.  The condition here is that someone is trying
>> to open a database named 'relay:'.  They likely do not have a file
>> named like that, so they will get a slightly different error while
>> trying to open it.  Should be clear enough for a user, I hope.
>>
>> What do you think?
> 
> Thanks for the clarification, this seems fine to me.
> 

I acked all other patches in the series and I was waiting for this
discussion to reach a conclusion.  Now that it did:

Acked-by: Dumitru Ceara <[email protected]>

Regards,
Dumitru


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to