In some situations, we want to execute an external shell command
in a non-blocking way. Similar to 'system', but without waiting for
the child to complete. We also want to close all file descriptors
ahead of the exec() and filter + modify the environment.
Change-Id: Ib24ac8a083db32e55402ce496a5eabd8749cc888
Related: OS#4332
The new OSMO_DEPRECATED_OUTSIDE macro is similar to the existing
OSMO_DEPRECATED_OUTSIDE_LIBOSMOCORE, but allows to override the
deprecation message.
Let's use it to suspend deprecation warnings related to:
- gsm48_decode_bcd_number(),
- osmo_ecu_fr_conceal(),
- osmo_ecu_fr_reset(),
as they're intentionally used in scope of the library.
Change-Id: I1b0eff1396776900c1286e41da3aee3ff78b326e
Provide string escaping that
- returns the required buffer size, so it can be used with OSMO_STRBUF_APPEND().
- uses C compatible string constant escaping sequences.
This is intended as a replacement for all previous osmo_escape_str* and
osmo_quote_str* API. It pains me that I didn't get them right the first nor the
second time:
- The buffer functions do not return the chars needed, which is required for
allocating sufficient memory in the *_c versions of the functions.
- Because of that, these functions are accurately usable for
OSMO_STRBUF_APPEND(), producing truncated strings, for example when dumping a
GSUP message.
- They do not use the C equivalent string constant escaping: for some reason I
thought "\15" would be valid, but it should be "\x0f".
If I could, I would completely drop those mislead implementations ... but
backwards compat prohibits that.
A previous patch already provided internal static functions that accurately
return the required buffer size. Enhance these to also support C compatible
string escaping, and use them as implementation of the new functions:
osmo_escape_cstr_buf()
osmo_escape_cstr_c()
osmo_quote_cstr_buf()
osmo_quote_cstr_c()
In the tests for these, also test C string equivalence.
Naming: from API versions, it would be kind of logical to call them
osmo_escape_str_buf3() and osmo_escape_str_c2(). Since these anyway return a
different escaping, it makes sense to me to have distinct names instead.
Quasi missing are variants of the non-C-compatible weird legacy escaping that
return the required buffer size, but I refrain from adding those, because we
have enough API cruft as it is. Just always use these new cstr variants.
Change-Id: I3dfb892036e01000033dd8e7e4a6a0c32a3caa9b
Follow up for patch I3cf150cc0cc06dd36039fbde091bc71b01697322
osmo_sockaddr_str_{from,to}_32n actually use host byte order. Deprecate these
and introduce a more accurately named version ending in h.
Change-Id: Ic7fc279bf3c741811cfc002538e28e8f8560e338
When finding a char in a string, I want to be able to limit the search area by
size, not only by nul terminator.
Change-Id: I48f8ace9f51f8a06796648883afcabe3b4e8b537
Provide a common implementation for foo_name_c() functions that base on
foo_name_buf() functions.
char *foo_name_c(void *ctx, example_t arg)
{
OSMO_NAME_C_IMPL(ctx, 64, "ERROR", foo_name_buf, arg)
}
Rationale: the most efficient way of composing strings that have optional parts
or require loops for composition is by writing to a ready char[], and this in
turn is easiest done by using OSMO_STRBUF_* API. Using such a basic name string
implementation which typically returns a length, I often want a more convenient
version that returns a char*, which can just be inlined in a "%s" string format
-- crucially: skipping string composition when inlined in a LOGP(). This common
implementation allows saving code dup, only the function signature is needed.
Why not include the function signature in the macro? The two sets of varargs
(1: signature args, 2: function call args) are hard to do. Also, having an
explicit signature is good for readability and code grepping / ctags.
Upcoming uses: in libosmocore in the mslookup (D-GSM) implementation
(osmo_mslookup_result_name_c()), and in osmo_msc's codec negotiation
implementation (sdp_audio_codecs_name_c(), sdp_msg_name_c(), ...).
I54b6c0810f181259da307078977d9ef3d90458c9 (libosmocore)
If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 (osmo-msc)
Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322
It's hard to figure out what color logging categories have with those ANSI
color code strings. Instead, define these OSMO_LOGCOLOR_* constants.
Naming: commonly, the logging.h header has the "LOG" prefix in the name, but it
seems saner to include the OSMO_ prefix: it seems too likely that some
libosmocore user somewhere already has defined "LOGCOLOR_RED" somewhere.
Change-Id: I03b6b1f73ae7ee61d37ff921e071a3d0881d3e9a
Currently planned user: for Distributed GSM in osmo-hlr: setting per-MSC
service addresses in VTY: replace/remove existing entries.
osmo_sockaddr_str_cmp() is useful to catch identical resulting IP addresses,
regardless of differing strings (e.g. '0::' and '::' are equal but differ in
strings).
Change-Id: I0dbc1cf707098dcda75f8e07c1b936951f9f9501
The format prints IP:port separated by a colon, which of course is confusing
when the IPv6 address itself contains mostly colons. The new format adds square
braces.
cafe:face::1:42 -> [cafe:face::1]:42
The IPv4 format remains unchanged:
1.2.3.4:42
Change-Id: I161f8427729ae31be0eac719b7a4a9290715e37f
In a multi-threaded environemnt, it's likely that each thread will have
its own, distinct set of file descriptors that it wants to watch.
Hence, let's make the osmo_fd_* functions configure not one global
list of file descriptors, but a thread-local list of file descriptors.
Change-Id: I5082ed3e500ad1a7516e1785bc57e008da2fac9a
Often, an IP address of 0.0.0.0 is considered an unset value (for clients
requiring a server address; not for listening on "any").
osmo_sockaddr_str_is_set() does return false when the port is 0, but there is
no simple way to tell whether the IP address is actually set to a server
address.
Add osmo_sockaddr_str_is_nonzero() to return false if:
- the port is zero, or
- the IP address is zero (0.0.0.0 or ::0), or
- the IP address cannot be parsed.
A practical use example: osmo-msc so far accepts an RTP IP address of 0.0.0.0
as valid. I noticed when trying to trigger error handling from a ttcn3 test.
osmo-msc can use this function to reject invalid addresses from MGCP messages.
Related: I53ddb19a70fda3deb906464e1b89c12d9b4c7cbd (osmo-msc)
Change-Id: I73cbcab90cffcdc9a5f8d5281c57c1f87b2c3550
This is a simpler and more general solution to the problem so far solved by
osmo_fsm_term_safely(true). This extends use-after-free fixes to arbitrary
functions, not only FSM instances during termination.
The aim is to defer talloc_free() until back in the main loop.
Rationale: I discovered an osmo-msc use-after-free crash from an invalid
message, caused by this pattern:
void event_action()
{
osmo_fsm_inst_dispatch(foo, FOO_EVENT, NULL);
osmo_fsm_inst_dispatch(bar, BAR_EVENT, NULL);
}
Usually, FOO_EVENT takes successful action, and afterwards we also notify bar.
However, in this particular case, FOO_EVENT caused failure, and the immediate
error handling directly terminated and deallocated bar. In such a case,
dispatching BAR_EVENT causes a use-after-free; this constituted a DoS vector
just from sending messages that cause *any* failure during the first event
dispatch.
Instead, when this is enabled, we do not deallocate 'foo' until event_action()
has returned back to the main loop.
Test: duplicate fsm_dealloc_test.c using this, and print the number of items
deallocated in each test loop, to ensure the feature works. We also verify that
the deallocation safety works simply by fsm_dealloc_test.c not crashing.
We should probably follow up by refusing event dispatch and state transitions
for FSM instances that are terminating or already terminated:
see I0adc13a1a998e953b6c850efa2761350dd07e03a.
Change-Id: Ief4dba9ea587c9b4aea69993e965fbb20fb80e78
This API will be used by libosmo-netif's osmo_stream for SCTP sockets,
which in turn will be used by libosmo-sccp to support multi-homed
connections.
Related: OS#3608
Change-Id: Ic8681d9e093216c99c6bca4be81c31ef83688ed1
log_enable_multithread() enables use of locks inside the
implementation. Lock use is disabled by default, this way only
multi-thread processes need to enable it and suffer related
complexity/performance penalties.
Locks are required around osmo_log_target_list and items inside it,
since targets can be used, modified and deleted by different threads
concurrently (for instance, user writing "logging disable" in VTY while
another thread is willing to write into that target).
Multithread apps and libraries aiming at being used in multithread apps
should update their code to use the locks introduced here when
containing code iterating over osmo_log_target_list explictly or
implicitly by obtaining a log_target (eg. osmo_log_vty2tgt()).
Related: OS#4088
Change-Id: Id7711893b34263baacac6caf4d489467053131bb
This is useful for timers expected to have a range of valid or expected
values.
Validation is done at runtime when timer values are set by the app or by
the user through the VTY.
Related: OS#4190
Change-Id: I4661ac41c29a009a1d5fc57d87aaee6041c7d1b2
Nothinh really forbids this case, it's totally fine allocating all space
of msgb as headroom. osmo-pcu actually does that in
gprs_rlcmac_ul_tbf::snd_ul_ud().
Related: OS#4029
Change-Id: Ibe05d08e3169a2603e891f76682a3b352a93ec7a
This list is really not needed by applications and currently only used
internally in logging.c and logging_vty.c.
Change-Id: I5dca069512bfcd0826194427c5482fad8bfd0232
Global symbol osmo_log_info is declared in logging.c as non-const,
because it is modified. As soon as logging_internal.h is included into
logging.c, the compiler warns about osmo_log_info being declared twice
differently.
Change-Id: Iea961c3caeb12ddf60c99d4dca644bb9ab538767
I missed code review, so here are my comments in form of a follow-up patch
for Id56a1226d724a374f04231df85fe5b49ffd2c43c.
- Fix 'as_unit' arg name to 'val_unit' as in the C file and API doc.
- Explain rounding-up behavior of value conversion in API doc.
- Use osmo_tdef_get_entry() instead of a loop.
Related: OS#4190
Change-Id: Ia91c2f17e40fb9e79ffa5a7f28ce9c3605664402
This API is already useful for users willing to set a given timer to a
given value. It will also contain code later that checks for value being
inside valid range for that timer.
Related: OS#4190
Change-Id: Id56a1226d724a374f04231df85fe5b49ffd2c43c
The pointless '(R)->ip?' condition of the previous commit made me want to
protect against R == NULL instead.
Change-Id: Ie2f47ad8ae585aaf67a6476c67f8e014820a72bc
Since (R)->ip is a char[], it is always non-NULL. The (x ? : "") condition is
completely pointless. Remove it.
Change-Id: I13ed06776a784cfa99bbdfca2bb4dfe12913a1ec
Rather than having applications maintain their own talloc cotexts,
let's offer some root talloc contexts in libosmocore. Let's also
make them per thread right from the beginning. This will help
some multi-threaded applications to use talloc in a thread-safe
way.
Change-Id: Iae39cd57274bf6753ecaf186f229e582b42662e3
The intention of osmo_tdef_get()'s val_if_not_present argument was to return a
default timeout, or to optionally abort the program for missing timer
definitions if the default timeout is < 0. This was the case in the original
implementation of this API in osmo-bsc, but in the migration to libosmocore,
the argument was by accident changed to an unsigned type. In consequence, the
assertion in the implementation that was intended to abort the program seemed
bogus to coverity, and was fixed by removal in
I7a544d2d43b83135def296674f777e48fe5fd80a -- the wrong direction, as is obvious
from the API doc for osmo_tdef_get().
Note that osmo-bsc master passes -1 in various places and expects the
program-abort behavior that was missing from the libosmocore implementation.
Change the val_if_not_present argument to a signed type, and revert removal of
the assertion, so that passing -1 has the effect described in the API doc:
program abort on missing timer definition.
This bug was not detected because it is hard to write tests that expect a
program abort to happen, hence no tests for this API feature exist.
Related: OS#4152
Change-Id: Ie61c3c85069916336e6dbd91a2c16f7634816417
This reverts commit 4e284b6379.
Unfortunately, some projects such as OsmoMSC, OsmoBTS and OpenBSC
do contain OSMO_ASSERT statements without a semi colon. Thus,
this change causes compilation errors when building them.
Please note that only the OSMO_ASSERT's definition is reverted,
while changes to other files (adding missing semicolons) are kept.
Change-Id: I6da4d7397d993f6c1af658cb5ae1e49c92a1b350
When using `OSMO_ASSERT(exp);` clang will warn about
an empty expression because the semi colon was superflous.
Use do {} while (0) to enfore the need of a semi colon.
This might break other test.
Change-Id: I2272d29a81496164bebd1696a694383a28a86434
the DEBUG macro name and ARRAY_SIZE macro function are frequently
used in other projects. If these projects also use libosmocore,
the macros will be redefined. This also generates a warning message
during compilation.
Not redefining the macros removes the warning message and possible
(but unlikely) mis-redefinition.
Change-Id: I0ba91eae8eacc5542d1647601b372e417ed1713c
So far, the public API of osmo_fsm only allowed integral seconds as
timeout. Let's change that to milli-seconds in order to cover more
use cases.
This introduces
* osmo_fsm_inst_state_chg_ms()
* osmo_fsm_inst_state_chg_keep_or_start_timer_ms()
Which both work exactly like their previous counterparts without the _ms
suffix - the only difference being that the timeout parameter is
specified in milli-seconds, not in seconds.
The value range for an unsigned long in milli-seconds even on a 32bit
platform extends to about 48 days.
This patch also removes the documentation notice about limiting the
maximum value to 0x7fffffff due to time_t signed-ness. We don't use
time_t but unsigned long.
Change-Id: I35b330e460e80bb67376c77e997e464439ac5397
We often compose FSM instance IDs from context information, for example placing
an MSISDN string or IP:port information in the FSM instance id, using
osmo_fsm_inst_update_id_f(). This fails if any characters are contained that
don't pass osmo_identifier_valid(). Hence it is the task of the caller to make
sure only characters allowed in an FSM id are applied.
Provide API to trivially allow this by replacing illegal chars:
- osmo_identifier_sanitize_buf(), with access to the same set of illegal
characters defined in utils.c,
- osmo_fsm_inst_update_id_f_sanitize() implicitly replaces non-identifier
chars.
This makes it easy to add strings like '192.168.0.1:2342' or '+4987654321' to
an FSM instance id, without adding string mangling to each place that sets an
id; e.g. replacing with '-' to yield '192-168-0-1:2342' or '-4987654321'.
Change-Id: Ia40a6f3b2243c95fe428a080b938e11d8ab771a7
To be able to append an escaped or quoted string using
OSMO_STRBUF_APPEND_NOLEN(), the function signature must have the buf and len as
first args, like most other *_buf() functions.
Add osmo_escape_str_buf2() and osmo_quote_str_buf2() to match this signature.
A recent patch [1] has changed the return value of osmo_escape_str_buf() to
char*, removing the const. However, the functions may return const strings,
hence re-add the const. The new signatures always return the non-const buffer.
To avoid code duplication, implement osmo_quote_str_buf() and
osmo_escape_str_buf() by calling the new functions.
I decided to allow slight changes to the behavior for current osmo_escape_str()
and osmo_escape_str_buf(), because impact on callers is minimal:
(1) The new implementation uses OSMO_STRBUF_*, and in consequence
osmo_quote_str() no longer prints an ending double quote after truncated
strings; Before, a truncated output was, sic:
"this string is trunca"
and now this becomes, sic:
"this string is truncat
I decided to not keep the old behavior because it is questionable to begin
with. It looks like the string actually ended at the truncation boundary
instead of the reason being not enough space in the output buffer.
(2) The new osmo_escape_str_buf2() function obviously cannot pass-thru an
unchanged char* if no escaping was needed. Sacrifice this tiny optimization
feature to avoid code duplication:
- it is an unnoticeable optimization,
- the caller anyway always passes a string buffer,
- the feature caused handling strings and buffers differently depending on
their content (i.e. code that usually writes out strings in full length
"suddenly" truncates because a non-printable character is contained, etc.)
I considered adding a skip_if_unescaped flag to the osmo_quote_str_buf2()
function signature, but in the end decided that the API clutter is not worth
having for all the above reasons.
Adjust tests to accomodate above changes.
[1] 4a62eda225
Ibf85f79e93244f53b2684ff6f1095c5b41203e05
Change-Id: Id748b906b0083b1f1887f2be7a53cae705a8a9ae
In OSMO_STRBUF_APPEND, use local variable names that are less likely to shadow
other local variables: prefix with _sb_.
In OSMO_STRBUF_APPEND, add a check to add to .pos only if it is not NULL.
Add OSMO_STRBUF_APPEND_NOLEN(), which works for function signatures that don't
return a length. This is useful for any osmo_*_buf() string writing functions,
so that these write directly to the strbuf.
Change-Id: I108cadf72deb3a3bcab9a07e50572d9da1ab0359
Move from a static implementation in tdef_vty.c to utils.c, I also want to use
this in osmo-msc.
The point is that the telnet VTY allows unambiguous partly matches of keyword
args. For example, if I have a command definition of:
compare (apples|oranges)
then it is perfectly legal as for the vty parser to write only
compare app
One could expect the VTY to then pass the unambiguous match of "apples" to the
parsing function, but that is not the case.
Hence a VTY function implementation is faced with parsing a keyword of "app"
instead of the expected "apples".
This is actually a very widespread bug in our VTY implementations, which assume
that exactly one full keyword will always be found. I am now writing new
commands in a way that are able to manage only the starts of keywords.
Arguably, strstr(a, b) == a does the same thing, but it searches the entire
string unnecessarily.
Change-Id: Ib2ffb0e9a870dd52e081c7e66d8818057d159513
Add global flag osmo_fsm_term_safely() -- if set to true, enable the following
behavior:
Detect osmo_fsm_inst_term() occuring within osmo_fsm_inst_term():
- collect deallocations until the outermost osmo_fsm_inst_term() is done.
- call osmo_fsm_inst_free() *after* dispatching the parent event.
If a struct osmo_fsm_inst enters osmo_fsm_inst_term() while another is already
within osmo_fsm_inst_term(), do not directly deallocate it, but talloc-reparent
it to a separate talloc context, to be deallocated with the outermost FSM inst.
The effect is that all osmo_fsm_inst freed within an osmo_fsm_inst_term()
cascade will stay allocated until all osmo_fsm_inst_term() are complete and all
of them will be deallocated at the same time.
Mark the deferred deallocation state as __thread in an attempt to make cascaded
deallocation handling threadsafe. Keep the enable/disable flag separate, so
that it is global and not per-thread.
The feature is showcased by fsm_dealloc_test.c: with this feature, all of those
wild deallocation scenarios succeed.
Make fsm_dealloc_test a normal regression test in testsuite.at.
Rationale:
It is difficult to gracefully handle deallocations of groups of FSM instances
that reference each other. As soon as one child dispatching a cleanup event
causes its parent to deallocate before fsm.c was ready for it, deallocation
will hit a use-after-free. Before this patch, by using parent_term events and
distinct "terminating" FSM states, parent/child FSMs can be taught to wait for
all children to deallocate before deallocating the parent. But as soon as a
non-child / non-parent FSM instance is involved, or actually any other
cleanup() action that triggers parent FSMs or parent talloc contexts to become
unused, it is near impossible to think of all possible deallocation events
ricocheting, and to avoid running into freeing FSM instances that were still in
the middle of osmo_fsm_inst_term(), or FSM instances to enter
osmo_fsm_inst_term() more than once. This patch makes deallocation of "all
possible" setups of complex cross referencing FSM instances easy to handle
correctly, without running into use-after-free or double free situations, and,
notably, without changing calling code.
Change-Id: I8eda67540a1cd444491beb7856b9fcd0a3143b18
To prevent re-entering osmo_fsm_inst_term() twice for the same osmo_fsm_inst,
add flag osmo_fsm_inst.proc.terminating. osmo_fsm_inst_term() sets this to
true, or exits if it already is true.
Update fsm_dealloc_test.err for illustration. It is not relevant for unit
testing yet, just showing the difference.
Change-Id: I0c02d76a86f90c49e0eae2f85db64704c96a7674
We have a habit of returning static buffers from some functions,
particularly when generating some kind of string values. This is
convenient in terms of memory management, but it comes at the expense
of not being thread-safe, and not allowing for two calls of the
related function within one printf() statement.
Let's introduce _c suffix versions of those functions where the
caller passes in a talloc context from which the output buffer shall
be allocated.
Change-Id: I8481c19b68ff67cfa22abb93c405ebcfcb0ab19b
Provide a common implementation of use counting that supports naming each user
as well as counting more than just one use per user, depending on the rules the
caller implies.
In osmo-msc, we were originally using a simple int counter to see whether a
connection is still in use or should be discarded. For clarity, we later added
names to each user in the form of a bitmask of flags, to figure out exactly
which users are still active: for logging and to debug double get / double put
bugs. This however is still not adequate, since there may be more than one CM
Service Request pending. Also, it is a specialized implementation that is not
re-usable.
With this generalized implementation, we can:
- fix the problem of inadequate counting of multiple concurrent CM Service
Requests (more than one use count per user category),
- directly use arbitrary names for uses like __func__ or "foo" (no need to
define enums and value_string[]s),
- re-use the same code for e.g. vlr_subscr and get fairly detailed VLR
susbscriber usage logging for free.
Change-Id: Ife31e6798b4e728a23913179e346552a7dd338c0
For handling RTP IP addresses and ports, osmo-mgw, osmo-bsc and osmo-msc
so far have their own separate shims and code duplication around
inet_ntoa(), htons(), sockaddr conversions etc. Unify and standardize
with this common API.
In the MGW endpoint FSM that was introduced in osmo-bsc and which I
would like to re-use for osmo-msc (upcoming patch moving that to
osmo-mgw), it has turned out that using char* IP address and uint16_t
port number types are a convenient common denominator for logging,
MGCP message composition and GSM48. Ongoing osmo-msc work also uses this
for MNCC.
This is of course potentially useful for any other IP+port combinations
besides RTP stream handling.
Needless to say that most current implementations will probably stay
with their current own conversion code for a long time; for current
osmo-{bsc,msc,mgw} work (MGW endpoint FSM) though, I would like to move
to this API here.
Change-Id: Id617265337f09dfb6ddfe111ef5e578cd3dc9f63
We have a number of static buffers in use in libosmo*. This means
the related functions are not usable in a thread-safe way. While
we so far don't have many multi-threaded programs in the osmocom
universe, the static buffers also prevent us from calling the same
e.g. string-ify function twice within a single printf() call.
Let's make sure there's an alternative function in all those cases,
where the user can pass in a caller-allocated buffer + size, and make
the 'classic' function with the static buffer a wrapper around that
_buf() variant.
Change-Id: Ibf85f79e93244f53b2684ff6f1095c5b41203e05
osmo_escape_str_buf() used to have the somewhat odd semantics that
if no escaping was needed, it would return the original pointer without
making any copy to the output buffer. While this seems like an elegant
optimization, it is a very strange behavior and it works differently
than all of our other *_buf() functions. Let's unify the API and
turn osmo_escape_str_buf() into a strlcpy() if no escaping is needed.
Change-Id: I3a02bdb27008a73101c2db41ac04248960ed4064
Doxygen was confused by duplicated documentation for both
definition and declaration of rate_ctr_for_each_counter().
Moreover, both variants contained some mistakes.
Let's avoid this duplication and keep the only (corrected) one.
Change-Id: Icca2d4a95bd5f96ae85a86909ec90fb8677cacf3
core/msgb.h:414: warning: argument 'msgb' of command @param is not
found in the argument list of
msgb_pull_to_l2(struct msgb *msg)
core/msgb.h:399: warning: argument 'msgb' of command @param is not
found in the argument list of
msgb_pull_to_l3(struct msgb *msg)
core/msgb.h:351: warning: argument 'msgb' of command @param is not
found in the argument list of
msgb_push_u16(struct msgb *msg, uint16_t word)
core/msgb.h:361: warning: argument 'msgb' of command @param is not
found in the argument list of
msgb_push_u32(struct msgb *msg, uint32_t word)
core/msgb.h:341: warning: argument 'msgb' of command @param is not
found in the argument list of
msgb_push_u8(struct msgb *msg, uint8_t word)
Change-Id: I5d660933ecfa89c631319eccf9e3d5c1986ec8ff
Thanks to the following Doxygen warning:
msgb.h:XXX: warning: The following parameters of
msgb_eq_l2(msg1, msgb2, len) are not documented:
parameter 'msgb2'
parameter 'len'
it was discovered that parameter 'len' is not required at all.
It basically doesn't make any sense to pass any length value,
because it can be calculated using msgb_length().
Let's drop this parameter. Given that this part of the API was
broken so far (see I1079d629abdb8770eef6be7341e586a933cd9cca),
it should be more or less safe to do this.
Change-Id: Icd9b72eb6bfa9628ff1ed2f948b57058551a4328