Hi

On Wed, Aug 25, 2021 at 10:24 AM Arne Schwabe <a...@rfc2549.org> wrote:

>
> >
> > +static bool
> > +management_callback_remote_entry(void *arg, unsigned *count, char
> **remote)
> > +{
> > +    assert(arg);
> > +    assert(count);
> > +
> > +    struct context *c = (struct context *) arg;
> > +    struct connection_list *l = c->options.connection_list;
> > +    bool ret = true;
> > +
> > +    if (!remote) /* query is for the count of entries */
> > +    {
> > +        *count = l->len;
> > +    }
> > +    else if (*count < l->len) /* the query is for entry with index =
> count */
> > +    {
> > +        struct connection_entry *ce = l->array[*count];
> > +        const char *proto = proto2ascii(ce->proto, ce->af, false);
> > +
> > +        /* space for output including 2 commas and a nul */
> > +        int len = strlen(ce->remote) + strlen(ce->remote_port) +
> strlen(proto) + 2 + 1;
> > +        char *out = malloc(len);
> > +        check_malloc_return(out);
> > +
> > +        openvpn_snprintf(out, len, "%s,%s,%s", ce->remote,
> ce->remote_port, proto);
> > +        *remote = out;
> > +    }
> > +    else
> > +    {
> > +        ret = false;
> > +        msg(M_WARN, "Invalid arguments in management query for remote
> entry: count = %u", *count);
> > +    }
> > +    return ret;
> > +}
> > +
>
> I am not sure mixing two related but different functions into one
> function has any real advantages. A function that just returns the count
> would simplify that function and this function.
>

Makes sense. v2 is coming.


> > +static void
> > +man_remote_entry_count(struct management *man)
> > +{
> > +    unsigned count = 0;
> > +    if (man->persist.callback.remote_entry)
> > +    {
> > +
> (*man->persist.callback.remote_entry)(man->persist.callback.arg, &count,
> NULL);
>
> The return value is not used here, this might upset some compilers.
>

The above will fix that too.


>
> > +        msg(M_CLIENT, ">REMOTE-ENTRY-COUNT:%u", count);
> > +    }
> > +    else
> > +    {
> > +        msg(M_CLIENT, "ERROR: The remote-entry-count command is not
> supported by the current daemon mode");
> > +    }
> > +}
>
> At some point we should do a helper function that does this boilerplate
> code that we copy&paste here over and over.
>

Actually this else could be possibly eliminated as, in this case, the
callback is not conditionally compiled in. Unlike things like pkcs11-id
support. Will check and simplify.

Thanks,

Selva
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to