Attention is currently required from: pespin.

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

Change subject: core: guard TCP stats on availability of linux/tcp.h
......................................................................


Patch Set 2:

(2 comments)

File src/core/stats_tcp.c:

https://gerrit.osmocom.org/c/libosmocore/+/41922/comment/81aa7d6b_bed2da43?usp=email
 :
PS2, Line 25: #if !defined(EMBEDDED)
> You can probably remove this defined(EMBEDDED) block now, since it will be 
> covered by the ifdef HAVE […]
I see the point, but I'm a bit hesitant to drop the EMBEDDED guard purely
because of HAVE_LINUX_TCP_H.

With --enable-embedded this code path used to be disabled regardless of what
headers are present. If we remove the EMBEDDED guard now, an embedded build
could end up compiling it just because linux/tcp.h happens to be available in
the toolchain sysroot, which would be a behaviour change compared to before.

I'd prefer to keep the embedded decision explicit (either keep the EMBEDDED
guard or force HAVE_LINUX_TCP_H / the corresponding feature off when
--enable-embedded is used in configure.ac), and only use HAVE_LINUX_TCP_H to
guard the Linux-specific implementation details. This way embedded builds stay
consistent, and CI should remain happy.


https://gerrit.osmocom.org/c/libosmocore/+/41922/comment/cca1e99a_29dc69e8?usp=email
 :
PS2, Line 52: static struct osmo_tcp_stats_config s_tcp_stats_config = {
> This should also be inside the HAVE_LIJNUX_TCP_H block below.
I don't think this can be moved entirely under HAVE_LINUX_TCP_H.

The data structure is accessed unconditionally from other code paths:
- stats.c calls osmo_stats_tcp_set_interval(osmo_tcp_stats_config->interval)
- stats_vty.c reads/writes osmo_tcp_stats_config->{interval,batch_size} and
  prints them in the VTY config output

If the definition/allocation is moved under HAVE_LINUX_TCP_H, those callers
will end up dereferencing a NULL pointer at runtime, leading to a segfault.



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/41922?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: Ibcd00f15131b0291f0b10eca51401c518b77cc39
Gerrit-Change-Number: 41922
Gerrit-PatchSet: 2
Gerrit-Owner: Timur Davydov <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Sun, 25 Jan 2026 22:38:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <[email protected]>

Reply via email to