On erratic id in osmo_fsm_inst_update_id(), don't say "Attempting to allocate
FSM instance".
Escape the invalid id using osmo_quote_str().
Change-Id: I770fc460de21faa42b403f694e853e8da01c4bef
Since alloc relies on osmo_fsm_inst_update_id() to set the name, never skip
that.
In osmo_fsm_inst_alloc(), we allow passing a NULL id, and in
osmo_fsm_inst_update_id(), we set the name without id if id is NULL.
Change-Id: I6d6b09a811b82770818f19b189a57d9fc4a8133b
strcmp() *must not* be passed NULL pointers, or we hit:
../../../src/libosmocore/src/fsm.c:123:8: runtime error: null pointer passed as argument 2, which is declared to never be null
ASAN:DEADLYSIGNAL
(Or, alternatively, a segfault.)
If any of the search string or an FSM instance's name string should be NULL,
simply never match.
Technically, an FSM should never have a NULL name, but a current bug actually
allows this (pass NULL id to alloc), which will be addressed by an upcoming
patch. To test for it, we need to first make sure this here doesn't segfault.
Change-Id: I2e5f82c06d1a4727bd93e955366e3b62b2df1b32
Rationale: with osmo_escape_str(), you get the escaped contents of the string,
but not so graceful handling of NULL strings. The caller needs to quote it, and
for NULL strings not quote it.
osmo_quote_str() is like osmo_escape_str() but always quotes a non-NULL string,
and for a NULL string returns a literal NULL, i.e. it should (tm) give the
exact C representation of a string.
That's useful in testing, to show exactly what char* situation we have, without
jumping through hoops like
if (str)
printf("\"%s\"", osmo_escape_str(str, -1));
else
printf("NULL");
Copy the unit test for osmo_escape_str() and adjust. To indicate that the
double quotes are returned by osmo_quote_str(), use single quotes in the test
printf()s.
I considered allowing to pick the quoting characters by further arguments, but
that complicates things: we'd need to escape the quoting characters. Just
hardcode double quotes like C.
Change-Id: I6f1b3709b32c23fc52f70ad9ecc9439c62b02a12
fix for some spelling issues found by lintian
Signed-off-by: Thorsten Alteholz <osmocom@alteholz.de>
Change-Id: I69976ecae6939d9ff51bfe4ce7374890c6563b82
After investigating osmo-msc showing this log message and looking at the
code, it's a bit difficult to find out what's going on in the code:
socket.c:224 unable to bind socket: (null):0: Protocol not supported
The root cause was not yet found, but probably SCTP is not enabled in
the kernel of the host running it.
The cod eis most probably failing during socket() and not due to bind
error as the log says, so let's print an error if socket() fails.
Then, if setsockopt fails, we want to still keep trying in case an extra
addr was offered by addrinfo_helper. It is definetly wrong to continue
if setsockopt fails, because then we are skipping the bind(), which is a
fundamental part of what osmo_sock_init2 does.
Then, let's print the bind error when it really happens, and re-write
the extra log at the end if we reach the point at which no suitable addr
is found.
Change-Id: I1854422ad92dadf33ed4d849e15c0380c3bf1626
The CTRL interface has a ctrl_cmd_def_* API that allows deferring a CTRL
command reply until later. However, the command handling currently fails to
acknowledge this and deallocates the struct ctrl_cmd anyway.
Fix: in struct ctrl_cmd, add a defer pointer to be populated by
ctrl_cmd_def_make(). A cmd thus marked as deferred is not deallocated at the
end of command handling. This fix needs no change in calling code.
(Another idea was to return a different code than CTRL_CMD_HANDLED when the
command is to be deferred, but that would require adjusting each user of
ctrl_cmd_def_make(). The implicit marking is safer and easier.)
Show that handling deferred commands is fixed by adjusting the expectations of
ctrl_test.c's test_deferred_cmd() and removing the now obsolete exit_early
label.
One symptom of the breakage is that osmo-bts-sysmo crashes when asked to report
a trx's clock-info, which is aggravated by the fact that the sysmobts-mgr does
ask osmo-bts-sysmo for a clock-info.
The crash appears since Id583b413f8b8bd16e5cf92a8a9e8663903646381 -- it looked
like just fixing an obvious memory leak, which it did as shown by the unit
test, but deferred ctrl commands actually relied on that leak. Both fixed now.
Related: OS#3120
Change-Id: I24232be7dcf7be79f4def91ddc8b8f8005b56318
If either an INVOKE, either a RETURN_RESULT component has the
data with incorrect length (see Annex A, 3GPP TS 04.80), the
whole message is probably incorrect.
Let's drop such messages instead of silent truncation.
Change-Id: I2a169b0b84aa26ea2521edd55ff005c27ae6d808
As it was already documented before, the 'ss_request' struct has
a rudiment of deprecated 'ussd_request' struct - the 'ussd_text'
field. It represents the data either of an INVOKE component,
either of a RETURN_RESULT component, encoded as ASCII in case
if DCS is 0x0f (i.e. decoded by the code itself), otherwise
raw bytes 'as is'.
Previously, there was no possibility to distinguish between
ASCII and raw bytes with different DCS. Moreover, the payload
decoding is not desired in some cases.
Let's introduce the new fields, which will carry the raw
unmodified payload, its length and DCS (Data Coding Scheme).
Change-Id: Ia193d175021e145bb3b131290231f307dbefc64a
libosmocore has no value strings for BSSMAP cause codes yet.
- Add value strings for BSSMAP cause codes and a function
to retrieve them
Change-Id: I313dd8d7b06374e1e35ddc18b7a42562d9e25d45
Related: OS#1609
Deprecate osmo_init_logging() for the benefit of adding an explicit talloc
context argument to new function osmo_init_logging2(). Pass a ctx to
log_init() instead of hardcoded NULL.
Before now, *all* of our code uses a NULL ctx for logging, which amounts to
talloc "leaks" hit by address sanitizer builds on newer gcc (e.g. gcc 7.3.0 on
debian 9).
This commit helps fixing "leaks" detected in e.g. osmo-bsc unit tests by a
sanitize build with gcc (Debian 7.3.0-12) 7.3.0.
Change-Id: I216837780e9405fdaec8059c63d10699c695b360
Fix GCC version 7.3.0 (Debian 7.3.0-12) compiler warning:
../../../../src/libosmocore/src/vty/command.c: In function ‘write_config_file’:
../../../../src/libosmocore/src/vty/command.c:2741:2: error: null destination pointer [-Werror=format-overflow=]
sprintf(config_file_tmp, "%s.XXXXXX", config_file);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Check agains NULL after each _talloc_zero() in write_config_file().
While at it, add a comment explaining why we don't use talloc_asprintf() instead.
Change-Id: I7bdc52afe724c1d21f419fe49a6e2ebca9420969
Use non-deprecated API to decode encode in gsm0808_enc_cell_id_list2().
Adjust gsm0808_test.c to now expect the correct results instead of previous
failure.
Change-Id: I1ce78883995e0d484368046b69db5afb2b4adc97
The speech codec defaults are not correct. The defaults recommended
in 3GPP TS 28.062, Table 7.11.3.1.3-2 are limited by 3GPP TS 48.008,
Section 3.2.2.103. Some defaults are actually reserved for future
use. Also the endianess of the 16 bit values is reversed.
- correct values so that they match the specification
- transmit bytes in the correct endianess
Change-Id: I6c3a34d39a375d71c4128fd38f06629e8b98b100
If the name stays the same the log messages will still log with the old
id. Since we can now change the id we need to update the name as well.
NULL as id was allowed before so we should allow that as well.
Change-Id: I6b01eb10b8a05fee3e4a5cdefdcf3ce9f79545b4
This is a more modern way of printing the Abis OML Formatted Object
Header, without assuming that it would be used in a log statement
or prescribing the log level to be used.
Change-Id: I9b2c2afec28882b817d104d5b062651ade7aadd8
Since commit bf383a1d83 tlv_parse()
will return the first occurrence of a repeated IE. Add a test to
verify this behaviour. This test passes with the current code and
fails if bf383a1d83 is reverted.
While here, fix lies in documentation about the return value of tlv_parse()
and fix a typo in another comment.
Change-Id: I041f38548c5e4236920991d6c681c1c1e04de9ca
Related: OS#2904
The implementation was entirely broken, reading data from wrong offsets
and always writing to the first element of the decoded list.
Also, add a new test for this function which found the problems.
Change-Id: If0fafbc7171da2a3044bfa9a167208a1afa1c07b
Related: OS#2847
Depends: Ife4e485e2b86c6f3321c9700611700115ad247b2
Cell ID lists with CI were misparsed because parse_cell_id_ci_list()
failed to report the amount of consumed bytes to its caller.
Also add a regression test which uncovered the bug.
Change-Id: Ife4e485e2b86c6f3321c9700611700115ad247b2
Depends: If6b941720de33dca66b6b1aa2cb95a3275708b7f
Related: OS#2847
This makes gsm0808_dec_cell_id_list() properly decode 3-digit MNCs.
Add a test which encodes/decodes a LAI_AND_LAC list with 3-digit MNCs.
Change-Id: If6b941720de33dca66b6b1aa2cb95a3275708b7f
Related: OS#2847
The cell ID list decoder merged in 11a4d9dd91
has a bug which was introduced part-way through the review process in
gerrit at https://gerrit.osmocom.org/#/c/6509/
When Neels suggested "why not just {...}id_list[MAXLEN] once?" I changed
the cell identifier list from a union of arrays to an array of unions.
After this change, elements smaller than the largest type in the union
were not laid out consecutively in memory anymore. E.g. uint16_t lac
values now occur at offsets of sizeof(id_list[0]) instead of offsets
of sizeof(uint16_t).
The problem is that I forgot to adjust the decoder accordingly, so the
decoder writes to the wrong offsets and returns cell identifier lists
which appear to contain uninitialized values when read back by API
consumers.
I found this problem while adding new regression tests to libosmocore to
test encoding and decoding. This commit adds one such tests for LAC list
decoding, which failed due to the above bug. I plan to write more tests,
however because this first test already uncovered a severe issue I chose
to submit a fix now and work on additional tests in later commits.
Change-Id: Ie1a5a9d858226be578cf11a03cf996d509bd51fb
Related: OS#2847
Global and LAI+LAC cell IDs were being misparsed due to an off-by-one.
This code was incorrectly converted from osmo-bsc, where an additional
offset of one byte was needed to skip the cell identifier field.
In libosmocore, these parsing routines receive a buffer pointer which
is already positioned at the start of the cell identifier field.
Change-Id: I7f3e8ace26176e9cbfe2542961d2a95662aa4d97
Related: OS#2847
Introduce gsm0808_dec_cell_id_list2() with supports additional types of
cell identifier lists. The new parsing routines are based on similar
routines used by the paging code in osmo-bsc's osmo_bsc_bssap.c.
Likewise, introduce gsm0808_enc_cell_id_list2() with support for the
same additional types of cell identifier lists.
The old API using struct gsm0808_cell_id_list is deprecated.
The previous definition was insufficient because it assumed that all
decoded cell ID types could be represented with a single uint16_t.
It was declared in a GSM protocol header (gsm/protocol/gsm_08_08.h)
despite being a host-side representation of data in an IE.
The only user I am aware of is in osmo-msc, where this struct is used
for one local variable. osmo-msc releases >= 1.1.0 make use of this API.
While here, fix a small bug in a test:
test_gsm0808_enc_dec_cell_id_list_bss() set the cell ID type to 'LAC'
but obviously wants to use type 'BSS'.
Change-Id: Ib7e754f538df0c83298a3c958b4e15a32fcb8abb
Related: OS#2847
An internal symbol '_talloc_zero' of talloc library was used
during a msgb allocation. This is not actually good because:
- it may be removed or modified by talloc developers;
- the behaviour may be changed by talloc developers;
- it's marked as internal using 'underscore';
- there is public API to do the same.
So, let's use the public API.
Change-Id: I1080c9071e997944cc0f9fc3716129e9395437ad
Printing an error message when msgb allocation failed was initially
intended, but have been commented out for years. This would
facilitate the bug hunting process, especially on embedded
platforms with limited resources (e.g. amount of RAM).
The GLOBAL logging subsystem with FATAL level is used
for printing such messages.
Change-Id: I3e2d1beabd6936fc28a1ad664c083ff1698bb644
The MSGB API is not a part of OpenBSC anymore, so let's remove
dead includes, which were probably left here during the
migration process.
Change-Id: Ief562a6e5b220a84902f95862d67279f953ee726
In osmo_mnc_from_str() do not try to return some values even if the validation
fails; hence don't try to decode a NULL pointer. That whole idea was half-baked
and a can of worms to begin with.
Change-Id: Ibaaa128ac60b941a015a31134eb52aef56bc6e22
osmo-bsc and osmo-bts share enums and value strings to describe
feature data that is exchanged via OML (manufacturer id) on startup.
Also the functions to set and get the respecitive bits in the feature
bitvectors are in osmo-bsc and osmo-bts. This is a code duplication
and should be resolved.
- add enum osmo_bts_features (replaces enum gsm_bts_features)
- add osmo_bts_features_descs (replaces gsm_bts_features_descs)
- add osmo_bts_set_feature (replaces gsm_btsmodel_set_feature)
- add osmo_bts_has_feature (replaces gsm_btsmodel_has_feature)
Change-Id: Id0c35aef11aa49aa40abe7deef1f9dbd12210776
osmo_mnc_from_str() preserves leading zeros in the string and is useful for
VTY config parsing (osmo-bsc, osmo-msc, osmo-sgsn, osmo-pcu).
osmo_{plmn,mnc}_cmp() takes care of the slight intricacy of ignoring the 3-digit flag
if the MNC is anyway >99. Will be used by osmo-sgsn.git and osmo-bsc.git. (All
current users just care about identical MNC, but a proper cmp doesn't hurt.)
Change-Id: Ib7176b1d65a03b76f41f94bc9d3293a8a07d24c6
Enable representing three-digit MNC with leading zeros. The MNCs 23 and 023 are
actually different; so far we treated both as 23. Re-encode an incoming BCD or
string of 023 as it were, i.e. not dropping the leading zero as 23.
Break ABI compatibility by changing the size and ordering of structs
gprs_ra_id, osmo_plmn_id, osmo_cell_global_id, ... by adding an mnc_3_digits
flag.
Change ordering in gprs_ra_id because the canonical oder is {Mobile Country
Code, Mobile Network Code}, so have the mcc member first.
ABI compatibility cannot be maintained for struct gprs_ra_id, since it is a
direct member of structs bssgp_bvc_ctx and bssgp_paging_info, and even just
adding a flag to the end would cause ABI changes of those structs. Similarly,
osmo_plmn_id is a direct member of osmo_location_area_id, and so forth.
Add new API to set and read this additional flag to preserve leading zeros:
- osmo_plmn_to_bcd(), osmo_plmn_from_bcd() after
gsm48_mcc_mnc_to_bcd() and gsm48_mcc_mnc_from_bcd().
- gsm48_decode_lai2(), gsm48_generate_lai2() after
gsm48_decode_lai(), gsm48_generate_lai().
- gsm0808_create_layer3_2() after gsm0808_create_layer3() and gsm0808_create_layer3_aoip().
- various osmo_*_name() functions in gsm23003.h (osmo_rai_name() still in
gsm48.h close to struct gprs_ra_id definition). The amount and duplication of
these may seem a bit overboard, but IMO they do make sense in this way.
Though most code will soon see patches unifying the data structures used, in
some cases (vty, ctrl) they are required singled out. Without these
functions, the formatting ("%0*u", mnc_3_digits ? 3 : 2, mnc) would be
duplicated all over our diverse repositories.
In various log output, include the leading MNC zeros.
Mark one TODO in card_fs_sim.c, I am not sure how to communicate a leading zero
to/from a SIM card FS. The focus here is on the core network / BSS.
To indicate ABI incompatibility, bump libosmogsm and libosmogb LIBVERSIONs;
adjust debian files accordingly.
Implementation choices:
- The default behavior upon zero-initialization will be the mnc_3_digits flag
set to false, which yields exactly the previous behavior.
- I decided against packing the mnc with the mnc_3_digits field into a
sub-struct because it would immediately break all builds of dependent
projects: it would require immediate merging of numerous patches in other
repositories, and it would make compiling older code against a newer
libosmocore unneccessarily hard.
Change-Id: Id2240f7f518494c9df6c8bda52c0d5092f90f221
If no event names are defined for an FSM, show a placeholder
message which points out the problem instead of segfaulting.
Change-Id: I87457945a7b76aa052305c9c531722be1ea0c1d1
Related: OS#3007
Event names are displayed in VTY commands so all FSM should have them.
Print an error message if an FSM is registered without event names.
We could also return an error code, however at present no caller checks
the return value of osmo_fsm_register() so this would be pointless.
Add event names to the test FSM and update expected output accordingly.
Change-Id: I08b100d62b5c50bf025ef87d31ea39072539cf37
Related: OS#3008
For all other decode operations we report the BER, but not for the
RACH. This results in osmo-bts-trx not being able to report BER
to the higher layers, which is possible on other BTS backends.
Let's close this gap by introducing gsm0503_rach_ext_decode_ber()
and gsm0503_rach_decode_ber() with the usual n_errors / n_bits_total
arguments.
Change-Id: I2b1926a37bde860dcfeb0d613eb55a71271928c5
There is a desire to install osmo_fsm vty commands automatically in
a library context, rather than requiring every application which
directly or indirectly uses osmo_fsm to run osmo_fsm_vty_add_cmd().
However, the function install_element_ve() asserts that elements
about to be installed have not already been installed.
This means we cannot shift responsibility into a library context
without first making sure that osmo_fsm commands are only installed
once per combined application+library context, because applications
won't know which commands any of its libraries has already installed.
A simple solution is to use a global flag which is checked by
osmo_fsm_vty_add_cmd() before installing osmo_fsm commands, and
is set once the commands have been installed. This way, no harm
is done if osmo_fsm_vty_add_cmd() is called multiple times.
Change-Id: I10b0b1c1c1bf44c3b8eafc465c1ee06ea2590682
Related: OS#2967
This breaks all existing / older osmocom-bb builds, and hence
cannot be accpeted. See also https://gerrit.osmocom.org/#/c/6679
Related: OS#2985
This reverts commit 3c38e60cd5.
Change-Id: Icfc52ca4e5cbe3a444d98037d27fa101e3614e06
The function _osmo_fsm_inst_term() terminates all child FSMs befor
it calls fi->fsm_cleanup(). This prevents the cleanup callback to
perform last actions on the child FSMs (e.g.
osmo_fsm_inst_unlink_parent()).
- Since moving the cleanup callack to the beginning of the function
would alter the termination behavior and possibly cause malfunction
in already existing implementation that use OSMO fsm, a new
optional callback that is called immediately at the beginning of
the terminatopn process is added.
Change-Id: I0fdda9fe994753f975a658c0f3fb3615949cc8bb
Closes: OS#2915
gsmtap_sendmsg() does not free the msgb if it returns a failure rc, so the
callers must check the rc and free the msg.
Change-Id: I7cf64ed9b14247298ed8b4ab8735627f8235a499
If less than the msgb size was written by write(), we want to return -EIO.
Hence do not return zero when write() wrote zero bytes, return -EIO in that
case as well.
Previously, if write() returned zero, gsmtap_sendmsg() would return zero
*without* freeing the msg, hence neither would the (ideal) caller. So this
fixes a corner-case memleak.
Change-Id: I099ae1c663c018da5db884f7e9d52c45af3ed817
Not freeing on error does enable callers to try to re-send as well, so it is a
kind of useful feature, even though I find it likely for callers to either
forget about freeing the msg on error or double-free by accident...
I considered changing gsmtap_sendmsg() to always free, but since it is public
API, I chose to keep and document its current behavior properly instead. We
don't know what callers may exist out there.
Change-Id: Id3266ce36442024f16eaf6afa3f516d201930c41
Sometimes we want to create an FSM instance before we know its name. In
that case we should be able to update the id later.
Change-Id: Ic216e5b11d4440f8e106a297714f4f06c1152945
Neels Hofmeyr
Pau Espin
Philipp Maier
Harald Welte
Daniel Willmann