Re: [Openvpn-devel] [PATCH v3 1/3] Add remote-count and remote-entry query via management

2022-12-27 Thread Gert Doering
Hi,

On Mon, Dec 26, 2022 at 05:30:23PM -0500, Selva Nair wrote:
> > > +
> > > +#define min(a,b) ((a) < (b) ? (a) : (b))
> >
> > We already have min_int and min_uint in integer.h. For consistency I
> > would prefer using those instead of adding a macro.

Unfortunately, this is more than "just an extra macro" - it breaks our
MSVC builds due to the windows equivalent of -Werr and macro
redefinition...

https://github.com/OpenVPN/openvpn/actions/runs/3786194652/jobs/6436868937

D:\a\openvpn\openvpn\src\openvpn\manage.c(862,1): warning C4005: 'min': macro 
redefinition [D:\a\openvpn\openvpn\src\openvpn\openvpn.vcxproj]
D:\a\openvpn\openvpn\src\openvpn\manage.c(862,1): error C2220: the following 
warning is treated as an error 
[D:\a\openvpn\openvpn\src\openvpn\openvpn.vcxproj]

(not sure why it calls out "the following warning" when it talks about the
one before it? but anyway)

this came a bit unexpected, so it's been pushed out already...

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


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v3 1/3] Add remote-count and remote-entry query via management

2022-12-26 Thread Selva Nair
Hi

On Mon, Dec 26, 2022 at 2:45 PM Arne Schwabe  wrote:

> Am 08.09.21 um 00:31 schrieb selva.n...@gmail.com:
> > From: Selva Nair 
> >
> > Selecting the remote host via the management iterface
>
> *sigh* I reviewed the v2 instead of the v3 but the typo is still here :P
>
> > +static void
> > +man_remote_entry_count(struct management *man)
> > +{
> > +unsigned count = 0;
> > +if (man->persist.callback.remote_entry_count)
> > +{
> > +count =
> (*man->persist.callback.remote_entry_count)(man->persist.callback.arg);
>
> We could use C99 style here.
>
> > +msg(M_CLIENT, "%u", count);
> > +msg(M_CLIENT, "END");
> > +}
> > +else
> > +{
> > +msg(M_CLIENT, "ERROR: The remote-entry-count command is not
> supported by the current daemon mode");
> > +}
> > +}
> > +
> > +#define min(a,b) ((a) < (b) ? (a) : (b))
>
> We already have min_int and min_uint in integer.h. For consistency I
> would prefer using those instead of adding a macro.
>
>
> > +
> > +static void
> > +man_remote_entry_get(struct management *man, const char *p1, const char
> *p2)
> > +{
> > +ASSERT(p1);
> > +
> > +if (man->persist.callback.remote_entry_get
> > +&& man->persist.callback.remote_entry_count)
> > +{
> > +bool res;
> > +unsigned int from, to;
> > +unsigned int count =
> (*man->persist.callback.remote_entry_count)(man->persist.callback.arg);
> > +
> > +from = (unsigned int) atoi(p1);
> > +to = p2? (unsigned int) atoi(p2) : from + 1;
> > +
>
> We probably want a space after p2.
>
> I know this has been very long and we are very close to 2.6 so
>
> Acked-By: Arne Schwabe 
>

Thanks..


>
> Maybe we can do a follow up patch to fix the mentioned small issues.
>

Will do as a followup or in the rebased pachc depending what Gert asks for.

Would it be too much to ask for a review of this one too ?
https://patchwork.openvpn.net/project/openvpn2/patch/20210907223614.8574-1-selva.n...@gmail.com/

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


Re: [Openvpn-devel] [PATCH v3 1/3] Add remote-count and remote-entry query via management

2022-12-26 Thread Arne Schwabe

Am 08.09.21 um 00:31 schrieb selva.n...@gmail.com:

From: Selva Nair 

Selecting the remote host via the management iterface


*sigh* I reviewed the v2 instead of the v3 but the typo is still here :P


+static void
+man_remote_entry_count(struct management *man)
+{
+unsigned count = 0;
+if (man->persist.callback.remote_entry_count)
+{
+count = 
(*man->persist.callback.remote_entry_count)(man->persist.callback.arg);


We could use C99 style here.


+msg(M_CLIENT, "%u", count);
+msg(M_CLIENT, "END");
+}
+else
+{
+msg(M_CLIENT, "ERROR: The remote-entry-count command is not supported by 
the current daemon mode");
+}
+}
+
+#define min(a,b) ((a) < (b) ? (a) : (b))


We already have min_int and min_uint in integer.h. For consistency I 
would prefer using those instead of adding a macro.




+
+static void
+man_remote_entry_get(struct management *man, const char *p1, const char *p2)
+{
+ASSERT(p1);
+
+if (man->persist.callback.remote_entry_get
+&& man->persist.callback.remote_entry_count)
+{
+bool res;
+unsigned int from, to;
+unsigned int count = 
(*man->persist.callback.remote_entry_count)(man->persist.callback.arg);
+
+from = (unsigned int) atoi(p1);
+to = p2? (unsigned int) atoi(p2) : from + 1;
+


We probably want a space after p2.

I know this has been very long and we are very close to 2.6 so

Acked-By: Arne Schwabe 

Maybe we can do a follow up patch to fix the mentioned small issues.


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