Commit Graph

430 Commits

Author SHA1 Message Date
Harald Welte c6a8697800 Introduce helper functions for safe fork+exec of processes
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
2019-12-17 13:49:28 +01:00
Vadim Yanitskiy 87b51439c1 core/defs.h: introduce and use OSMO_DEPRECATED_OUTSIDE
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
2019-12-11 12:42:05 +00:00
Neels Hofmeyr 8a7eed50db add osmo_escape_cstr and osmo_quote_cstr
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
2019-11-24 19:59:35 +01:00
Neels Hofmeyr 951d32b817 osmo_sockaddr_str: deprecate osmo_sockaddr_str_*_32n()
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
2019-11-24 19:59:35 +01:00
Neels Hofmeyr 06356fd9c3 utils: add osmo_strnchr()
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
2019-11-24 19:59:35 +01:00
Neels Hofmeyr 63cb949ebc msgb_put: more elaborate logging of head/tailroom failure
Change-Id: I55b68098e1037c74ebe5faa86e34bd4494f5b726
2019-11-24 19:59:35 +01:00
Neels Hofmeyr 344776d251 fsm.h: add missing include of logging.h
Change-Id: I783bf0eb40b674fb6a77f7673563fdf156975f5a
2019-11-24 19:59:35 +01:00
Neels Hofmeyr 823073aa91 utils.h: add OSMO_NAME_C_IMPL() macro
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
2019-11-23 07:58:47 +00:00
Neels Hofmeyr e883de54f8 cosmetic: logging.h: fix comment s/levels/subsystems
Change-Id: I242a4a44649bc4dac055985ba8fd63b2f784ee6d
2019-11-23 07:58:47 +00:00
Neels Hofmeyr f2644aee55 logging.h: define ansi color constants
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
2019-11-23 07:58:47 +00:00
Neels Hofmeyr 002a51d731 add osmo_sockaddr_str_cmp()
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
2019-11-21 21:17:12 +01:00
Neels Hofmeyr d0e8d6b9b5 fix OSMO_SOCKADDR_STR_FMT for IPv6
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
2019-11-11 20:14:01 +01:00
Harald Welte 7a010b10f7 select: Make file descriptor lists per-thread
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
2019-11-07 10:24:50 +01:00
Neels Hofmeyr 2ceb758ba4 add osmo_sockaddr_str_is_nonzero()
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
2019-11-01 14:33:58 +01:00
Neels Hofmeyr 988f6d72c5 add osmo_fsm_set_dealloc_ctx(), to help with use-after-free
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
2019-10-29 16:46:04 +01:00
Pau Espin 3f464fc007 socket: Introduce API osmo_sock_init2_multiaddr()
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
2019-10-18 09:21:48 +00:00
Pau Espin d12f698dbb logging: Introduce mutex API to manage log_target in multi-thread envs
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
2019-10-09 14:19:52 +02:00
Pau Espin 0cbe8f0100 tdef: Introduce min_val and max_val fields
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
2019-10-07 13:14:14 +00:00
Pau Espin ec4b62705d msgb: Allow size==headroom in msgb_alloc_headroom*()
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
2019-09-26 17:46:30 +02:00
Pau Espin 7bfaba51b0 logging: Move extern declaration of osmo_log_target_list from logging.h to logging_internal.h
This list is really not needed by applications and currently only used
internally in logging.c and logging_vty.c.

Change-Id: I5dca069512bfcd0826194427c5482fad8bfd0232
2019-09-20 15:21:33 +00:00
Oliver Smith 210acc631f logging.h: add L1 SAPI related context and filter
First user is osmo-bts in I6b7bb2e1d61502b61214f854a4ec5cbb7267545b.

Related: OS#2356
Change-Id: I814cb3328d99faca9220adb5a80ffb934f219d7d
2019-09-18 11:23:50 +00:00
Pau Espin 070628065a logging_internal.h: Fix osmo_log_info definition
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
2019-09-17 18:23:07 +02:00
Neels Hofmeyr 9655ed5cf5 tdef: fixup osmo_tdef_set()
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
2019-09-11 01:55:38 +02:00
Pau Espin 77cd10f0db tdef: Introduce API osmo_tdef_set()
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
2019-09-07 22:29:06 +00:00
Neels Hofmeyr f89cbd0019 OSMO_SOCKADDR_STR_FMT_ARGS: guard against NULL pointer
The pointless '(R)->ip?' condition of the previous commit made me want to
protect against R == NULL instead.

Change-Id: Ie2f47ad8ae585aaf67a6476c67f8e014820a72bc
2019-08-30 14:09:42 +02:00
Neels Hofmeyr 079693d22f OSMO_SOCKADDR_STR_FMT_ARGS: remove useless condition
Since (R)->ip is a char[], it is always non-NULL. The (x ? : "") condition is
completely pointless. Remove it.

Change-Id: I13ed06776a784cfa99bbdfca2bb4dfe12913a1ec
2019-08-30 14:09:42 +02:00
Harald Welte 2d90611cb0 context: Add support for [per-thread] global talloc contexts
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
2019-08-27 13:43:31 +02:00
Neels Hofmeyr 989f01c406 osmo_tdef_get(): allow passing -1 as default timeout
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
2019-08-15 03:02:34 +02:00
Vadim Yanitskiy e2425485c4 Revert "utils.h: require a semi colon after OSMO_ASSERT"
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
2019-07-09 14:05:19 +07:00
Alexander Couzens 4e284b6379 utils.h: require a semi colon after OSMO_ASSERT
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
2019-07-08 13:23:45 +00:00
Kevin Redon 77021c7bec (minor) fix typo in comments
Change-Id: I697af428a2ea9a0ccd3f04ba8ec4664935ae29f8
2019-06-13 18:13:34 +02:00
Kevin Redon d1e220fb67 minor: don't redefine macros
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
2019-06-13 13:49:30 +00:00
Vadim Yanitskiy d9fc6041e7 core/utils: drop meaningless const from return value of osmo_luhn()
Change-Id: I085da06f31a0a6862ae2ba041fafc134cc240f7e
2019-06-12 11:29:06 +00:00
Harald Welte 7b74551b93 fsm: Allow millisecond granularity in osmo_fsm built-in timer
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
2019-06-05 10:01:20 +00:00
Daniel Willmann 0a1259ba87 Deprecate usage of osmo_counter_*
There's not many of those around any more, let's try to move them to
osmo_stat_items

Change-Id: If67f64c6ec7a3f3114c962df9db50107d9ea86e2
2019-05-19 07:18:04 +00:00
Alexander Couzens cc72cc45a4 add osmo_stat_item_inc/osmo_stat_item_dec to set it relative
Change-Id: Id2462c4866bd22bc2338c9c8f69b775f88ae7511
2019-05-07 13:20:57 +00:00
Neels Hofmeyr b480b74192 add identifier sanitation for setting FSM instance ids
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
2019-04-12 01:00:16 +02:00
Neels Hofmeyr ecef7ec3c3 add osmo_{escape,quote}_str_buf2() for standard args ordering
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
2019-04-12 01:00:16 +02:00
Neels Hofmeyr 8531d6695f tweak OSMO_STRBUF_APPEND(), add OSMO_STRBUF_APPEND_NOLEN()
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
2019-04-11 07:40:25 +02:00
Neels Hofmeyr d79ccc65f7 add osmo_str_startswith()
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
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 1f9cc01861 fsm: support graceful osmo_fsm_inst_term() cascades
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
2019-04-11 05:36:36 +00:00
Neels Hofmeyr 3b414a4adc fsm: add flag to ensure osmo_fsm_inst_term() happens only once
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
2019-04-11 05:36:36 +00:00
Harald Welte 179f35702e Add _c versions of functions that otherwise return static buffers
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
2019-04-10 22:42:32 +00:00
Neels Hofmeyr 0e8df1c7e4 add osmo_use_count API
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
2019-04-08 13:47:17 +00:00
Neels Hofmeyr 0c7826e9bd add osmo_sockaddr_str API
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
2019-04-08 13:47:17 +00:00
Harald Welte 4a62eda225 Add _buf() functions to bypass static string buffers
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
2019-04-03 18:03:14 +02:00
Harald Welte 98ed3393cd osmo_escape_str_buf: Always copy, don't return input string pointer
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
2019-03-29 16:42:25 +00:00
Vadim Yanitskiy c761044ecb Doxygen: fix documentation of rate_ctr_for_each_counter()
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
2019-03-27 08:56:21 +01:00
Vadim Yanitskiy 914a8ec5b3 core/msgb.h: fix incorrect Doxygen parameter description
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
2019-03-27 08:56:21 +01:00
Vadim Yanitskiy ba426e3b0c core/msgb.h: drop meaningless parameter of msgb_eq_* helpers
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
2019-03-27 08:56:21 +01:00