Attention is currently required from: laforge, neels, pespin.

Timur Davydov has posted comments on this change by Timur Davydov. ( 
https://gerrit.osmocom.org/c/libosmocore/+/41813?usp=email )

Change subject: Add Emscripten build support and JS callback logging backend
......................................................................


Patch Set 19:

(5 comments)

File configure.ac:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/ac292e99_fddbad83?usp=email
 :
PS16, Line 259: AM_CONDITIONAL(ENABLE_TUN, test "x$embedded" != "xyes" && test 
"x$emscripten" != "xyes")
> This will still probably need fixing in a separate patch like the NETNS stuff 
> you fixed
Agreed, this likely needs an additional fix. This should follow the same
approach as netns. I’ll address this in a follow-up patch.


File src/core/Makefile.am:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/e4e0753f_7b78d474?usp=email
 :
PS16, Line 87: if ENABLE_TUN
> This will probably need some sort of other fix? Why can't you add it when 
> building for emscripten?
Agreed, this likely needs an additional fix. This should follow the same
approach as netns. I’ll address this in a follow-up patch.


File src/core/osmo_io_internal.h:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/b0a4317c_9b0388a0?usp=email
 :
PS16, Line 5: #include "../config.h"
> This and the change below deserve a separate patch, feel free to submit 
> previously in the branch to  […]
Done, split out and submitted as a separate patch.


File src/core/stats_tcp.c:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/9a5e15b4_66cc0485?usp=email
 :
PS16, Line 198: #if !defined(__EMSCRIPTEN__)
> This should be fixed in a more generic way. […]
The build fails due to missing Linux-specific headers/types.

First, without any guards:
  stats_tcp.c:33:10: fatal error: 'linux/tcp.h' file not found
    #include <linux/tcp.h>

If I try to workaround this by guarding the include with `#if 
defined(__linux__)`,
the build still fails because the code below relies on Linux-only types/macros:
  stats_tcp.c:103:18: error: variable has incomplete type 'struct tcp_info'
  stats_tcp.c:112:56: error: use of undeclared identifier 'TCP_INFO'

So this can’t be fixed just by wrapping the include. We need a generic
configure-time feature/header check (e.g. for linux/tcp.h and/or the presence
of struct tcp_info / TCP_INFO) and conditionally build/enable the TCP_INFO
stats code based on that.

I’ll address this in a separate patch.


File src/vty/logging_vty.c:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/0dd72b47_c759ba86?usp=email
 :
PS16, Line 937: #if !defined(__EMSCRIPTEN__)
> what about leaving this in and let it fail during runtime if user tries to 
> use it?
Agreed. I disabled it only to avoid creating files on the Web side, but since
Emscripten provides an IndexedDB-backed filesystem, leaving it enabled and
failing at runtime is fine. I’ll update the patch



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/41813?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia8d5f4bb6570b5e055826f3a051e5e5896866e31
Gerrit-Change-Number: 41813
Gerrit-PatchSet: 19
Gerrit-Owner: Timur Davydov <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: neels <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-CC: laforge <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Fri, 23 Jan 2026 00:42:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <[email protected]>

Reply via email to