VLA issue with 'MG_ENABLE_TCPIP'

[gvanem]

Compiling with MSVC and -DMG_ENABLE_TCPIP=1, gives me this error in src/net_builtin.c:

net_builtin.c(288): error C2057: expected constant expression
net_builtin.c(288): error C2466: cannot allocate an array of constant size 0
net_builtin.c(288): error C2133: 'opts': unknown size

But why not simply define opts_maxlen as a pre-processor constant:

--- a/src/net_builtin.c 2025-04-09 11:55:26
+++ b/src/net_builtin.c 2025-04-09 12:53:12
@@ -284,8 +284,8 @@
                              (ifp->enable_req_sntp ? 1 : 0));
   size_t len = strlen(ifp->dhcp_name);
   size_t olen = 21 + len + extra + 2 + 1;   // Total length of options
-  uint8_t opts_maxlen = 21 + sizeof(ifp->dhcp_name) + 2 + 2 + 1;
-  uint8_t opts[opts_maxlen]; // Allocate options (max size possible)
+  #define OPTS_MAXLEN (21 + sizeof(ifp->dhcp_name) + 2 + 2 + 1)
+  uint8_t opts[OPTS_MAXLEN]; // Allocate options (max size possible)
   uint8_t *p = opts;
   assert(olen <= sizeof(opts));
   memset(opts, 0, sizeof(opts));

Besides, I had this issue with src/arch_win32.h messing up with the -DMG_ENABLE_TCPIP code:

.\src/net_builtin.c(924,52): error: too few arguments provided to function-like macro invocation
  924 |     bool up = ifp->driver->poll(ifp, expired_1000ms);
      |                                                    ^
.\src\arch_win32.h(85,9): note: macro 'poll' defined here
   85 | #define poll(a, b, c) WSAPoll((a), (b), (c))
      |         ^
1 error generated.

I fail to understand the reason for this. But patched it like so:

--- a/src/arch_win32.h 2024-11-14 14:53:41
+++ b/src/arch_win32.h 2025-04-09 13:02:15
@@ -77,12 +77,16 @@
 #define MG_SOCKET_TYPE SOCKET
 typedef unsigned long nfds_t;
 #if defined(_MSC_VER)
+#if (MG_ENABLE_TCPIP == 0)
 #pragma comment(lib, "ws2_32.lib")
+#endif
 #ifndef alloca
 #define alloca(a) _alloca(a)
 #endif
 #endif
+#if (MG_ENABLE_TCPIP == 0)
 #define poll(a, b, c) WSAPoll((a), (b), (c))
+#endif
 #define closesocket(x) closesocket(x)

(what has ws2_32.lib to do with built-in networking?)

So after these changes, this sample works fine on Windows:

pcap-driver.exe -v 5 -i "\Device\NPF_{6568F487-0016-453C-8C29-5C28DEDF27A0}"
430cfa 2 main.c:267:main                Opened interface \Device\NPF_{6568F487-0016-453C-8C29-5C28DEDF27A0}
430cfa 2 main.c:293:main                Init done, starting main loop
430cfa 3 net.c:198:mg_listen            1 0 http://0.0.0.0:8000
430cfa 3 net.c:198:mg_listen            2 0 https://0.0.0.0:8443
430cfa 1 net_builtin.c:214:onstatechang Link up
430d0a 3 net_builtin.c:324:tx_dhcp_disc DHCP discover sent. Our MAC: 02:00:01:02:03:77
4310e2 3 net_builtin.c:324:tx_dhcp_disc DHCP discover sent. Our MAC: 02:00:01:02:03:77
4314ca 3 net_builtin.c:324:tx_dhcp_disc DHCP discover sent. Our MAC: 02:00:01:02:03:77
4318b2 3 net_builtin.c:324:tx_dhcp_disc DHCP discover sent. Our MAC: 02:00:01:02:03:77
4319bc 3 net_builtin.c:303:tx_dhcp_requ DHCP req sent
431a0a 2 net_builtin.c:442:rx_dhcp_clie Lease: 86400 sec (90797)
431a0a 1 net_builtin.c:211:onstatechang Got IP
431a29 2 net_builtin.c:207:onstatechang READY, IP: 10.0.0.147
431a29 2 net_builtin.c:208:onstatechang        GW: 10.0.0.1
431a29 2 net_builtin.c:209:onstatechang       MAC: 02:00:01:02:03:77
...

[scaprile]

I fail to understand the reason for this. But patched it like so:

The preprocessor redefines poll to WSAPoll, so ifp->driver->poll(ifp, expired_1000ms) ends up being ifp->driver->WSAPoll(ifp, expired_1000ms) and is complaining that one argument is missing, as the macro takes 3.

Please check #3095 and confirm

(what has ws2_32.lib to do with built-in networking?)

It is beyond our expected usage to run our builtin stack over Windows, even for tests, as we run them over Linux of even BSD. The arch definition for w32 just assumes WinSock is the only option

[gvanem]

Please check #3095 and confirm

Not #define OPTS_MAXLEN = 21 + sizeof(ifp->dhcp_name) + 2 + 2 + 1;,
but #define OPTS_MAXLEN 21 + sizeof(ifp->dhcp_name) + 2 + 2 + 1

It is beyond our expected usage to run our builtin stack over Window

Very well, but pcap works fine on Windows.

[scaprile]

Oops... finger faster than brain.
I'm working on a fix for the second issue, you have just beat me on preparing the PR, stay tuned

[scaprile]

Please check #3095 again and confirm.
Please check #3096 , bear with me on this as I don't have your setup, please check WinSock and built-in TCP/IP. If you also want to check other options like lwIP or FreeRTOS-Plus-TCP, you're welcome. Let me know how it goes.

[scaprile]

@gvanem 👆 #3096 is correctly building and working on our CI/CD pipeline now.

[gvanem]

Please check #3096 ,

Looks OK.

[gvanem]

Not quite correct; a -DMG_ENABLE_TCPIP=1 still requires <windows.h> which
AFAICS is only included via <winsock2.h>. Hence:

--- a/src/arch_win32.h 2025-04-10 13:57:03
+++ b/src/arch_win32.h 2025-04-10 14:04:34
@@ -103,9 +103,9 @@
 #define MG_PATH_MAX FILENAME_MAX
 #endif

-#if MG_ENABLE_WINSOCK
 #include <winsock2.h>

+#if MG_ENABLE_WINSOCK
 #define MG_INVALID_SOCKET INVALID_SOCKET
 #define MG_SOCKET_TYPE SOCKET
 #define poll(a, b, c) WSAPoll((a), (b), (c))

E.g. MultiByteToWideChar(), FILETIME, GetSystemTimeAsFileTime() are needed from <windows.h>

[scaprile]

OK, thought you've tested that when you said "looks good"
It builds and runs with VC98 and VC22 but we don't test our stack there

Can't we just include windows.h instead of winsock2 when we are not going to use winsock ?

[gvanem]

OK, thought you've tested that when you said "looks good"

On first sight. I had difficulty testing when it was not committed to master.

Can't we just include windows.h instead of winsock2 when we are not going to use winsock ?

But then by default <winsock.h> will be included.

[scaprile]

On first sight. I had difficulty testing when it was not committed to master.

Just git checkout builtdoze, where builtdoze is the branch name. Then you can go back to normal by checking out master

Moved that include up, to also cover similar needs of VC98 when loading wincrypt.h. Let me know how it goes; works for me