Hi,
Thanks for looking at this.
On Mon, Apr 2, 2018 at 8:37 AM, Steffan Karger <stef...@karger.me> wrote:
>
> Hi,
>
> One comment based on stare-at-code only:
>
> On 12-03-18 02:17, selva.n...@gmail.com wrote:
> > @@ -636,6 +640,8 @@ find_certificate_in_store(const char *cert_prop,
> > HCERTSTORE cert_store)
> > }
> > if (!*++p) /* unexpected end of string */
> > {
> > + msg(M_WARN, "WARNING: cryptoapicert: error parsing <%s>.",
> > cert_prop);
> > + return NULL;
> > break;
> > }
>
> The break after the return looks a bit strange, maybe remove it?
Agreed, the break is dead code and should go. Will send a v2.
>
>
> Also, this looks like a somewhat unrelated fix. I would have personally
> preferred it in a separate patch (so we can e.g. backport it easily even
> if we decide not not backport the functional change).
The original did the search based on subject and hash separately,
which is now combined into a loop below this parsing chunk. So a
return, in place of break, is required here. The warning on parse error
is new (instead of silently returning NULL). But the end result of that
is still a FATAL error later in the code, as before.
I'm ok to leave out the warning from this patch, though..
Selva
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
