Hi,

On Mon, Jan 25, 2021 at 01:56:24PM +0100, Arne Schwabe wrote:
> Our "natural" place for this function would be ssl.c but ssl.c has a lot of
> dependencies on all kinds of other compilation units so including ssl.c into
> unit tests is near impossible currently. Instead create a new file ssl_util.c
> that holds small utility functions like this one.
> 
> Patch v2: add newline add the end of sll_util.h and ssl_util.c

Even if it already got an ACK, I find the function could benefit from a 
v3... "if we refactor, go all the way"

- change to early-return

  if (!peer_info || ((var_start = strstr(peer_info, var)) == NULL))
  {
      return NULL;
  }

- possibly split the assignment-and-compare if() into easier to read

  const char *var_start = strstr(peer_info, var);
  if (!var_start)
  {
      return NULL;
  }

- half the function has been converted to "var" and "var_start", and
  the rest still talks "char *ncp_ciphers_peer"... wat? - maybe that
  should be "char *value" (and "var" should be "key"?) or something.

- the v2 hunk has a newline-at-end-of-file mishap in ssl.c

> index 14c8116f..f59b409f 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -4201,4 +4201,4 @@ void
>  ssl_clean_user_pass(void)
>  {
>      purge_user_pass(&auth_user_pass, false);
> -}
> +}
> \ No newline at end of file

(this is a new "no newline"), while v2 fixes the other one).


On the plus side, I tested "make distcheck" on linux, and all the Makefile
bits and pieces are proper (we tend to break that for new C files...)

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
                             Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany                             g...@greenie.muc.de

Attachment: signature.asc
Description: PGP signature

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

Reply via email to