Attention is currently required from: Timur Davydov, neels.

pespin 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 5: Code-Review-1

(12 comments)

Commit Message:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/7bd15204_b1ed5902?usp=email
 :
PS5, Line 7: Add Emscripten build support and JS callback logging backend
Why is this needed? How is this used? What's the context? I'm missing a good 
description here for a feature is not obvious we need.


File include/osmocom/core/logging.h:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/34e2672b_8f00cd59?usp=email
 :
PS5, Line 285:  LOG_TGT_TYPE_WEB,       /*!< Web logging */
AFAIU this is not "web generic" but "emscripten" specific, so let's please 
rename to something more meaningful, like "_EMSCRIPTEN"


File src/core/netdev.c:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/2d7abf8a_b341e9b0?usp=email
 :
PS5, Line 67: #if (!EMBEDDED) && !defined(__EMSCRIPTEN__)
I'd say better do this logic inside configure.ac and add a new auto variable 
HAVE_NETDEV or whatever.

BTW, what's actually the poroblem here when building for emscripten? does it 
fail to compile? does it fail at runtime?


File src/core/osmo_io_internal.h:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/6a392a82_e3c785b2?usp=email
 :
PS5, Line 7: HAVE_LIBSCTP
> This comes from `config.h`, which is included below. […]
Indeed config.h should be at the top.


File src/core/serial.c:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/cb4121c4_ad9127ec?usp=email
 :
PS5, Line 27: #if !defined(__EMSCRIPTEN__)
Again, I'd say better add a HAVE_SERIAL or alike in configure.ac. And again, 
what's failing to build/run here exactly? This is the kind of stuff which 
should be also documented in the commit description.


File src/core/socket.c:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/15140efa_3b148e26?usp=email
 :
PS5, Line 2050: #ifdef HAVE_LIBSCTP
missing explanation for this change. It may be a separate patch.


https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/1b1ccc3c_082d86e4?usp=email
 :
PS5, Line 1928: #ifdef HAVE_LIBSCTP
Why this change? I'm lacking any rationale here. Also, looks like this may be a 
separate patch.


File src/vty/Makefile.am:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/4dfded30_dfe96217?usp=email
 :
PS5, Line 36: libosmovty_la_SOURCES += telnet_interface_dummy.c
This for sure also needs an explanation.


File src/vty/logging_vty.c:

https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/f909049f_7dedfa63?usp=email
 :
PS5, Line 907: #if !defined(__EMSCRIPTEN__)
this also needs explanation.


https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/49f5f516_50b53edb?usp=email
 :
PS5, Line 1038: DEFUN(cfg_log_web, cfg_log_web_cmd,
#if defined(__EMSCRIPTEN__)


https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/fdab9457_8f2390c4?usp=email
 :
PS5, Line 1047:                 vty_out(vty, "%% Unable to create WEB log for 
%s", VTY_NEWLINE);
this string end looks wrong


https://gerrit.osmocom.org/c/libosmocore/+/41813/comment/b0da8cfc_9bb5b867?usp=email
 :
PS5, Line 1067:                 vty_out(vty, "%% Unable to find WEB log target 
for %s", VTY_NEWLINE);
this string end looks wrong



--
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: 5
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: Timur Davydov <[email protected]>
Gerrit-Comment-Date: Mon, 12 Jan 2026 10:13:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: fixeria <[email protected]>

Reply via email to