Let's be smarter and call the status update callback iff the V.24
flagmask was actually changed.
Change-Id: I9626d3e737d4e072fa163115c4cdf9ee6ee0968e
Related: OS#4396
The mismatch for bcap_csd_2400_v22bis is expected, because octet 4
is not represented in 'struct gsm_mncc_bearer_cap' and the encoder
unconditionally hard-codes it to 0x88.
Change-Id: I4fc519c02b4fad8b0f40fa649d9de14b1183d10d
Related: OS#4396, OS#4394
The previous PDP-Type IE should have been a PDP-Address from the
start, since having only PDP-Type with no address is only a specific
case (dynamic addressing).
This becomes clear by looking at other similar protocols like:
* MAP: APN-Configuration IE has servedPartyIP-IP{v4,v6}-Address IEs
* Diameter S6b, 3GPP TS 29.272 7.3.35 APN-Configuration contains
Served-Party-IP-Address AVPs
* Diameter SWx, 3GPP TS 29.273 APN-Configuration.
* GTPv1C Ts 29.060 7.7.29 PDP Context containing PDP Address.
Since PDP-Type on its own really makes no sense, being it a special case
of PDP-Address, let's keep the IE by renaming it (keeping old name too
for API backward compat) and extend it to support lengths > 2 bytes.
Old implementation of libosmogsm gsup actually ignored lengths > 2
bytes, so we are safe acting against older implementations here, both
on the sending and receiving side on the wire.
The big drawback of this commit is that it breaks ABI compatibility due
to adding "struct osmo_sockaddr pdp_address[2];" to struct
osmo_gsup_pdp_info, which in turn affects shift of fields in struct
osmo_gsup_message. Unfortunately, there's not much that can be done to
improve the situation when adding the missing field, due to existing API
having the same struct for all messages. Ideally we'd have 1 union with
structs per message type inside, this way the ABI break would be far
less pronounced.
The GSUP test output change is becaue we now accept some of the len>2
cases for PDP-Type/Address IE which were being rejected since a couple
commits ago.
libosmogsm gsup code is now disabled in EMBEDDED mode, since it nows
depends on core/socket.h (struct osmo_sockaddr) which is not available
in EMBEDDED, and hence fails during build:
"""
In file included from /build/include/osmocom/gsm/gsup.h:45,
from /build/src/gsm/gsup_sms.c:28:
/build/include/osmocom/core/socket.h:15:10: fatal error: arpa/inet.h: No such file or directory
15 | #include <arpa/inet.h>
| ^~~~~~~~~~~~~
"""
Related: OS#6091
Change-Id: I775ff9c3be165d9f30d6ab55d03f99b6104eadd6
Having both fields in an uin16_t integer makes it difficult and
confusing for users for no good reason. Let's have separate fields for
each of them.
The new fields are defined so that they are ABI compatible with previous
uin16 field.
Change-Id: Ie31c6080c90e468c01186259f2c42621e39b5cc6
As documented in gsup.adoc, this field is expected to be 2 bytes.
This is only a intermediate step to showcase the related test scenarios
submitting IE with len > 2. The logic will be changed in a follow-up
patch when changing the IE to also encode/decode the missing Address
part.
Change-Id: I0d024a9a4fb10beeff39ac33a9d2ed02f88f4580
Let's make those E1/E2/E3 bit combinations publicly available in
form of a lookup table (key is enum osmo_v100_sync_ra1_rate).
Add convenience macros for setting and comparing these bits.
This lookup table will be used by osmocom-bb.git.
Change-Id: I6d2f8e250df31c233a2741163113dc07515409ae
Related: OS#4396
This ensures multithreaded logging attempts, in particular ones that do
nothing, do not hold the lock just for checking the level, which
interferes with other logging attempts.
Closes: OS#5818
Change-Id: I35f8dd9127dd6e7feae392094fd6b3ce2d32558d
ITU-T recommendation V.110 defines Terminal Adaptor (TA) functions
for the connection of Terminal Equipment (TE) having standard V-series
interfaces to the ISDN. This patch brings "software" implementation
of the TA to libosmoisdn.
The primary user for this soft-TA is the mobile-side implementation
of CSD (Circuit Switched Data) in osmocom-bb. CSD is heavily based
on V.110, which is not surprising given that GSM is a "wireless ISDN".
Nevertheless, this code will likely also be useful in the context
of retro-networking.
Similarly to the existing V.110 code in libosmoisdn, the present
implementation aims to be functional and correct, rather than
efficient in any way. It also has several limitations, which are
not critical for the CSD use case, but eventually may be a problem
for other use cases in the context of retro-networking.
Therefore, the V.110 TA API should be considered _unstable_,
and may be subject to change in the future.
+-------+ +------+ B-channel +------+ +-------+
| TE1 |------| TA |~~~~~~~~~~~~~~~| TA |------| TE2 |
+-------+ +------+ +------+ +-------+
TE (also known as DTE) is basically a computer, having a V-series
(usually RS-232) connection to TA (also known as DCE). The TA acts
like a regular analog modem, except that it is not performing any
kind of modulation or demodulation itself.
The TE-TA interface is implemented by the user supplied callback
functions, configured during the allocation of a TA instance:
* .rx_cb() - receive call-back of the application,
* .tx_cb() - transmit call-back of the application,
* .status_update_cb() - status line update call-back.
In addition to that, the application (TE) can interact with the
V.24 status lines (circuits) using the following API:
* osmo_v110_ta_{get,set}_status(),
* osmo_v110_ta_{get,set}_circuit().
The Rx and Tx between TE and TA is always driven by the TA itself,
as a result of an interaction with the lower layer implementing
the B-channel interface. There is currently no buffering and thus
no way for TE to initiate transmission or pull data on its own.
The TA-TA (B-channel) interface is implemented by the following
functions, which are meant to be called by the lower layer
transmitting and receiving V.110 frames over certain medium:
* osmo_v110_ta_frame_in() - indicate a received V.110 frame,
* osmo_v110_ta_frame_out() - pull a V.110 frame for transmission,
* osmo_v110_ta_[de]sync_ind() - indicate a synchronization event.
The lower layer is responsible for finding the synchronization
pattern (if needed), aligning to the frame boundaries, and doing
the V.110 frame coding.
The D-channel signalling is behind the scope of this module.
Initial (Work-in-Progress) implementation by Harald Welte,
completed and co-authored by Vadim Yanitskiy.
Change-Id: I5716bd6fd0201ee7a7a29e72f775972cd374082f
Related: OS#4396
This API predates commit 7b74551b9, which added support for millisecond
granularity to osmo_fsm. Let's do the same for the tdef FSM wrapper
API, allowing the millisecond precision without rounding-up to seconds.
Of course, this patch changes behavior of the existing API, but having
more precise state timeouts is not going to make the API user
experience worse.
The old behavior of using seconds is for kept for:
* OSMO_TDEF_CUSTOM -- still treated as if it was OSMO_TDEF_S.
* \param[in] default_timeout -- still expected to be in seconds.
Change-Id: I4c4ee89e7e32e86f74cd215f5cbfa44ace5426c1
Related: 7b74551b9 "fsm: Allow millisecond granularity in osmo_fsm built-in timer"
This commit fixes the following warning seen with CC=clang:
utils.c:376:6: warning: variable 'len' set but not used [-Wunused-but-set-variable]
int len = 0, offset = 0, ret, rem;
... and finally allows to build libosmocore with --enable-werror.
Change-Id: I0040ef20ba3fc53ee7ccefc4885170f333f80566
This code predates 2cbe25f4, adding osmo_strbuf API and so using its
own append-to-strbuf implementation. Let's use the new generic API.
Change-Id: Ifdfd18eeef6a0932995063259f9f179b22e781de
The member data[0] in struct osmo_ecu_state is used as an anchor to
attach private structs for a concrete ECU implementation. This works by
allocating more memory then struct osmo_ecu_state actually needs and
then using the excess memory to store the private struct of the concrete
ECU implementation.
However, this poses a problem since data[0] is at the end of the struct
it may land in an unaligned position. This also means that the struct we
store there is also unaligned.
We should fix this enclosing the public struct osmo_ecu_state into our
private struct fr_ecu_state. Then we can use container_of to cast from
osmo_ecu_state to fr_ecu_state and correct alignment is ensured as well.
Related: OS#6286
Change-Id: I28672856e8e8f47e04ffe09ee3e07b577108cdc7
bitvec.c:543:14: warning: variable 'pos' set but not used [-Wunused-but-set-variable]
unsigned i, pos = 0;
Change-Id: I17df6f9263bee06676309c00837f12220803c814
The following can be seen when building with CC=clang:
utils/utils_test.c:1239:2: runtime error: applying non-zero offset 99 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior utils/utils_test.c:1239:2 in
utils/utils_test.c:1241:3: runtime error: applying non-zero offset 99 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior utils/utils_test.c:1241:3 in
utils/utils_test.c:1242:2: runtime error: applying non-zero offset 99 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior utils/utils_test.c:1242:2 in
44. testsuite.at:274: 44. utils (testsuite.at:274): FAILED (testsuite.at:278)
This makes utils_test fail due to unexpected UBSan's output.
Even though passing NULL to the strbuf API is relatively safe, it makes
no sense and the API user should ensure that this never happens. And
so we should not be testing this case.
Change-Id: Icd2323e93ec64afc1822d48e5e1d090083edf539
This can be seen when building with CC=clang:
utils.c:150:22: runtime error: applying non-zero offset 100 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior utils.c:150:22 in
utils.c:150:33: runtime error: addition of unsigned offset to 0x000000000064 overflowed to 0x000000000063
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior utils.c:150:33 in
The *dst pointer may be NULL (e.g. bcd2str_test() is passing it).
This makes tests/utils/utils_test fail. Let's fix this.
Change-Id: I542aef1ac220891b6bbdb0c60c39232f0df0a43c
We're seeing tons of -Wsign-compare warnings since I merged 0f59cebf:
include/osmocom/core/utils.h: In function 'size_t _osmo_strbuf_char_count(const osmo_strbuf*)':
include/osmocom/core/utils.h:24:29: error: comparison of integer expressions of different
signedness: 'long int' and 'long unsigned int'
[-Werror=sign-compare]
24 | #define OSMO_MIN(a, b) ((a) >= (b) ? (b) : (a))
| ~~~~^~~~~~
include/osmocom/core/utils.h:309:16: note: in expansion of macro 'OSMO_MIN'
309 | return OSMO_MIN(sb->pos - sb->buf, sb->len - 1);
| ^~~~~~~~
Interestingly enough, this -Wsign-compare problem has always been the
case, even before commit 0f59cebf. And somehow this did not show up
when building libosmocore.git, but only when building C++ projects
(osmo-pcu and osmo-trx).
Perhaps it has something to do with how g++ compiles extern "C" code.
Change-Id: I8e396459409e4260b8715f9e890e8972d4609a31
Fixes: 0f59cebf "utils: improve readability of OSMO_STRBUF_CHAR_COUNT"
Similarly to OSMO_STRBUF_REMAIN, let's improve the code readability
by adding a static inline function. We should generally prefer using
static inline functions over macros, unless there is something that
only the proprocessor can do.
Change-Id: I71f24b87c13fd83952029171a6993f8da5e32e5b
Currently, OSMO_STRBUF_REMAIN would return a huge number if the given
strbuf has its .pos pointer set to NULL. This macro is safe against
the .buf pointer being NULL, but not against .pos being NULL.
Fix this by adding a static inline function (for the sake of code
readability) and handle .pos being NULL properly by returning length
of the buffer. Add a unit test.
Change-Id: I294a74a99c40995cf7fb5520d61f697d967be5a4
Extend the existing testing coverage to check per-state enter/leave
callbacks. An interesting behavior can be seen from the test output:
when allocating an FSM instance, the .onenter callback is not being
called for the initial FSM state (ST_NULL). Likewise, the .onleave
callback is not being called when free()ing an FSM instance.
Change-Id: I22edcf91375a09854f0dab1e2e02e034629310f7
The code so far only supported 240bit RLP frames; Add support for
576bit in this patch. We still only support versions 0+1 and not
version 2.
Change-Id: Idfdcabb19fe8733fb9c5ee76a39b0bf4cdf60c2c
- Use defines instead of repeating string literals
- Use enum for indexing same data everywhere consistently
Related: OS#5915
Change-Id: I11c926dd4125c6657ae3cd21d8038f161d9fd9c3
We should not be doing the actual logic inside the assert statements.
Change-Id: I3bbf4e602c25c5aaced609e9834d6b053688194d
Fixes: CID#274664, CID#274679
These values end up being used by API users of
osmo_sock_multiaddr_get_name_buf() and
osmo_multiaddr_ip_and_port_snprintf().
Change-Id: I18a0e1a652a3e8ef3e97154355eb1d07a14ef0bd
As outlined in the test case, we pull a total of 50 bits from the
transmitter in two rounds, pulling 25 bits at a time. In the default
8-N-1 configuration, 50 bits should ideally comprise 5 characters.
However, as observed, only a total of 4 characters are retrieved
from the application, leaving the remaining 10 bits (5 + 5) unused.
Change-Id: Ic2539681a4adf6c1822e0bc256e4c829813d0e21
This problem can only happen if the user is flush()ing the Rx buffer
manually by calling osmo_soft_uart_flush_rx(). Let's demonstrate it
in the unit test, so that we don't forget about it (add FIXME).
Change-Id: Iad932a505d6fd98360f90510651501f8708ff5d2
Coverity tells us that with the current logic it's possible (in theory)
that we may dereference NULL pointer in osmo_soft_uart_flush_rx(). This
is highly unlikely, because the Rx buffer gets allocated once when the
Rx is enabled and remains even after the Rx gets disabled. The Rx flags
cannot be anything than 0x00 before the Rx gets enabled.
Even though this NULL pointer dereference is unlikely, the Rx flushing
logic is still not entirely correct. As can be seen from the unit test
output, the Rx callback of the application may be called with an empty
msgb if the following conditions are both met:
a) the osmo_soft_uart_flush_rx() is invoked manually, and
b) a parity and/or a framing error has occurred previously.
We should not be checking suart->rx.flags in osmo_soft_uart_flush_rx(),
since this is already done in suart_rx_ch(), which is calling it.
Removing this check also eliminates a theoretical possibility of the
NULL pointer dereference, so we're killing two birds with one stone.
- Do not check suart->rx.flags in osmo_soft_uart_flush_rx().
- Add a unit test for various flush()ing scenarios.
Change-Id: I5179f5fd2361e4e96ac9bf48e80b99e53a7e4712
Fixes: CID#336545
This is a convenience helper to reetrieve the whole set of remote
addresses and call getsockopt() on them, making it easy for users to
analyse the full set of remote addresses of a socket simply providing an
fd.
Related: SYS#6636
Change-Id: I3e1c84526b006baff435bbbca49dc6cf7d201cf5
In the _output_buf() we explicitly initialize only the 'buf' and 'len'
fields of the struct osmo_strbuf, leaving the 'pos' field implicitly
initialized to NULL. Later, in this function, 'sb.pos' is passed to
ctime_r() and strlen(), leading to a NULL pointer dereference (segfault)
in certain scenarios.
This situation can occur when color logging is disabled or when
a specific logging subsystem has no associated color. Any application
using libosmocore's logging API would crash with the following config:
log stderr
logging filter all 1
logging timestamp 1
logging color 0
Fix this by initializing the 'pos' field explicitly.
Change-Id: I7ec9badf525e03e54e10b725d820c636eaa3fd1c
Fixes: d71331bc "logging: fix nul octets in log output / use osmo_strbuf"
Fixes: CID#336550
The goto tag was wrong, probably due to a copy-paste mistype while
reimplementing the function.
Closes: Coverity CID#336546
Change-Id: I06b810fde7bf750fcb42d6d9e6223883e26f5f0b
According to Coverity, we check return value of this function in
all other cases except this one (9 out of 10 times), so let's add
the missing assert(), just to be sure.
Change-Id: I675f4089cc990be5fcda792276b6808742f6f0d7
Fixes: CID#336557
If the given queue is empty, queue->list.next points to &queue->list.
Current implementation would call llist_del() on the queue's llist_head,
decrement queue->current_length (which will be 0), and return a pointer
to &queue->list to the caller. This is completely wrong.
- Use the existing item_dequeue(), which does exactly what we need.
- Do not decrement the current_length if nothing was dequeued.
- Uncomment code in the unit test, we should not crash anymore.
Change-Id: I63094df73b166b549616c869ad908e9f4f7d46d1
Fixes: CID#336557