Attention is currently required from: Timur Davydov, neels.

Patch set 5:Code-Review -1

View Change

12 comments:

• Commit Message:

  • Patch Set #5, 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:

  • Patch Set #5, 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:

  • Patch Set #5, 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:

  • Patch Set #5, 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:

  • Patch Set #5, 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:

  • Patch Set #5, Line 2050: #ifdef HAVE_LIBSCTP

    missing explanation for this change. It may be a separate patch.

  • Patch Set #5, 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:

  • Patch Set #5, Line 36: libosmovty_la_SOURCES += telnet_interface_dummy.c

    This for sure also needs an explanation.

• File src/vty/logging_vty.c:

  • Patch Set #5, Line 907: #if !defined(__EMSCRIPTEN__)

    this also needs explanation.

  • Patch Set #5, Line 1038: DEFUN(cfg_log_web, cfg_log_web_cmd,

    #if defined(__EMSCRIPTEN__)

  • Patch Set #5, Line 1047: vty_out(vty, "%% Unable to create WEB log for %s", VTY_NEWLINE);

    this string end looks wrong

  • Patch Set #5, Line 1067: vty_out(vty, "%% Unable to find WEB log target for %s", VTY_NEWLINE);

    this string end looks wrong

To view, visit change 41813. To unsubscribe, or for help writing mail filters, visit settings.