Hi,

thanks for the review.

On Fri, Sep 11, 2020 at 09:03:43PM +0200, David Sommerseth wrote:
> On 11/08/2020 12:44, Gert Doering wrote:
> > This is a new "samples" plugin which does not do many useful things,
> > besides
[..]
> > +sample-client-connect.o: sample-client-connect.c
> > +
> > +# This directory is where we will look for openvpn-plugin.h
> > +CPPFLAGS=-I../../../include
> > +
> > +CC=gcc
> 
> If you don't set this; CC will normally be 'cc', which is normally linked to
> gcc on Linux systems.
> 
> > +CFLAGS=-O2 -Wall -Wno-unused-variable -g
> 
> I would probably skip the -Wno-unused-variable and rather not declare unused
> variables.

Yeah, not sure how that one slipped in.  I *did* try to make it warning-free,
so, next version is coming :-)

> > +    rl->name = strdup("config");
> 
> I'm getting a lot of "warning: implicit declaration of function 
> ???strdup???;" and
>  "warning: assignment to ???char *??? from ???int??? makes pointer from 
> integer
> without a cast" compiler warning on all of these strdup() calls.
> 
> We do use strdup() in the main openvpn code, but that code includes config.h,
> which contains #define _GNU_SOURCE 1.  This removes this compiler warning.
> 
> This is on RHEL-7 with both gcc-4.8 and gcc-9.3.

Not sure what you are hitting there.  According to "man strdup()", 
inclusion of <string.h> should be fully sufficient, without any extra
declarations.  But maybe it's a -std=... thing to tell GCC what
C language level is desired?

I developed and tested this on a gentoo linux, with gcc-9.2.0, and it's 
*not* throwing strdup() warnings.

> Otherwise, the code looks reasonable and it works.  The log file does not
> include the pushed echo statement (can be enabled in options.c:5286).  The
> management interface shows the pushed echo message.

Yeah, the "echo" directive is sort of weird.  It's blanked out of PUSH
messages as well...  I do wonder what James had in mind here, or what 
AS/Connect are using it for :-)

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