| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Place id and name testing in its separate section, test_id_api().
Add a test that actually allocates an FSM instance with a NULL id, which is
allowed, but uncovers a bug of an unset FSM instance name. osmo_fsm_inst_name()
falls back to the fsm struct's name on NULL, but osmo_fsm_inst_find_by_name()
fails to match if the instance's name is NULL (and until recently even
crashed). Show this in fsm_test.c with loud comments.
Add test to clear the id by passing NULL.
Add test for setting an empty id.
Add test for setting an invalid identifier (osmo_identifier_valid() == false).
Change-Id: I646ed918576ce196c395dc5f42a1507c52ace2c5
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
In fsm_test.c, we have FSM instance cleanup after the select main loop, but we
exit(0) in the timer cb; hence the final code is never called.
Rather clean up the instance and hence also test that, by using a global flag
to exit the main loop upon timeout.
Adjust expected stderr output.
BTW, in a subsequent commit, I want to move the fsm instance id testing to
below the main loop, to more clearly group the tested bits.
Change-Id: Ia47811ffcc1bd68d2630c86be7ab98fc1f338773
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
| |
Ironically, when deprecating osmo_init_logging() in
I216837780e9405fdaec8059c63d10699c695b360, I forgot to change the callers
within libosmocore itself, i.e. in the various regression tests.
Change-Id: Ia36c248f99353d5baaa2533f46a2f60a8579bdf8
|
|
|
|
|
|
| |
See explanations in previous commit.
Change-Id: I4889e777d8627fdfb52c97ab3ab353b6ed34aab2
|
|
|
|
|
|
| |
See explanations in previous commits.
Change-Id: Ib2f7577b9f498ae9d388ed1f79f6ca0ec6f09664
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
| |
Add logging to root ctx, add msgb ctx to root ctx, free wqueue to simulate the
msgb being sent, and assert final talloc size.
Change-Id: Ief3d5e7b6c4d781b3854e230e45a67d5281b94cd
|
|
|
|
| |
Change-Id: I3a711f5c974b7f56e27b333d390d1a706fb57007
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
| |
Handling a deferred command currently deallocates the struct ctrl_cmd upon
exiting the initial command handling, while it should actually stay around for
the asynchronous/deferred handling of the ctrl command.
Show the current bug by means of a ctrl test. The test will be adjusted to
expect the correct result when the bug is fixed in a subsequent commit
(I24232be7dcf7be79f4def91ddc8b8f8005b56318).
Change-Id: Ibbc847fc583bdd8e5e53a008258805e634ea12b4
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
| |
Change-Id: I22144dd759e0c1bb1965574b7ddd676606e5d6b7
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
| |
This is quite confusing, I'm surprised that it has not been flagged
before...
Change-Id: I3dc07290579949891e481675d493e5a2ea6d0aed
|
|
|
|
|
|
|
|
| |
FSMs with identical names confuse the script, so at least warn about them.
IMHO we should also have different names for each FSM, so not bothering to make
the script safe against identical naming.
Change-Id: I4fe7e85d2fdf23e2ba964f3b47be1954edc63cef
|
|
|
|
|
|
|
| |
If an FSM transitions to a state that actually belongs to a different FSM,
print an error and mark it red, instead of exiting with exception.
Change-Id: I73d95a0c65ca1ea586ad55234610671a53d6220f
|
|
|
|
|
|
|
|
|
|
| |
Do some more mad hacking to only detect event triggers if event names actually
appear in if() or case: statements. Also combine events in consecutive 'case'
statements.
When will this madness stop?
Change-Id: Iabaaab57f0d1687607eaaa4e09c7c2439fbd7a25
|
|
|
|
|
|
|
| |
Most arrows draw state transitions, but some want to show that an event is
delivered to a state. Mark those with a "halfopen" arrow head.
Change-Id: Ib07380894a6d513896c9135f175ecbf653a23eec
|
|
|
|
|
|
|
| |
When some edges have no label, it is sometimes hard to interpret which label
belongs to which edge. Adding a '-' default label clarifies the edge labeling.
Change-Id: I3a10b615288107e8fc12ffdbbe0099cf51abe94f
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Hacked as it is, fsm-to-dot is capable of detecting action functions
transitioning to states that are not allowed according to the FSM definition
struct.
Draw those in red and output a warning.
Found these osmo-bsc gscon errors with this patch:
ERROR: gscon_fsm_active() triggers a transition to ST_WAIT_HO_COMPL, but this is not allowed by the FSM definition
ERROR: gscon_fsm_wait_ho_compl() triggers a transition to ST_WAIT_MDCX_BTS_HO, but this is not allowed by the FSM definition
Related: OS#3109
Change-Id: Ic6319a958b3c7247510c1930bac8b02b95f9dcf2
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Strip comments from function bodies before matching on event names.
In osmo-bsc's gscon FSM, there often are event names in comments. The naive
parsing of fsm-to-dot.py mistakes these as events causing state transitions,
but the comments are just explaining how states interact.
Makes me reconsider parsing the C with clang instead, but I got away with a
dirty hack once more.
Change-Id: I56d70ae14d363f7ca655dced16d93d795b3f940d
|
|
|
|
|
|
|
| |
In osmo-bsc's new gscon FSM, there is an osmo_fsm_inst_state_chg() from
ST_ACTIVE to ST_ACTIVE. Avoid an exception triggering on this simple fact.
Change-Id: I420c7be84e3af555cc5e8bddbff7261013348375
|
|
|
|
| |
Change-Id: I196033e44d50ebb73cf9b44cbdc94a2b8b4f98ce
|
|
|
|
|
|
|
| |
Recent commit I77cd4b9142510c6914298b720d9c19ab68f9ebef left an obsolete
assertion around. It is already done in the if-body now.
Change-Id: I1bb2ea363e8a9d86b24338df3584abc93ebc6dd4
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
| |
Change-Id: I763b18d2922701ad97382269747ff0cbb7bd657b
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The test currently sets the MCC by a hex value, which is a weird choice. The
MCC gets BCD'd and hence we will see the decimal values 1:1 in the encoded
octets as hex digits. Using hex as input obscures that:
Right now it sets mcc = 0x123, which is actually 291 in decimal, and we hence
see "92 .1" in the expected BCD result. Using 0x124 in the test source actually
makes it hard to see where the 0x123 went.
Change the MCC to decimal notation (123, 124, 125) and adjust the expected
encoded output.
Change-Id: I973835c54a90fefe50d2b3581324d12556715f58
|
|
|
|
|
|
|
|
|
|
|
| |
I am going to ehance the test to actually include leading-zero MNC, but first I
would like to simplify how the test source struct is initialized, before I edit
around in it.
Also, when the memcmp() fails, print hexdumps of expected and actual result for
comparison. I needed it to figure out a test failure, might as well keep it.
Change-Id: I77cd4b9142510c6914298b720d9c19ab68f9ebef
|
|
|
|
|
|
|
|
|
|
|
|
| |
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 bf383a1d83661af26ccd6521c49b655fb22531d4 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 bf383a1d83661af26ccd6521c49b655fb22531d4 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
|
|
|
|
|
|
|
|
|
| |
Add a regression test which encodes and decodes a cell identifier
list of type CELL_IDENT_WHOLE_GLOBAL.
Change-Id: Ie633d9e55c6a30555c0153d35aaf9f982d6e0088
Depends: If0fafbc7171da2a3044bfa9a167208a1afa1c07b
Related: OS#2847
|
|
|
|
|
|
|
|
|
|
|
| |
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 11a4d9dd91216fe353e94bfdbbab53bc4f891c0d
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
|
|
|
|
|
|
|
| |
Note that the jenkins_arm.sh passes various CFLAGS and potentially overwrites
./configure internal CFLAGS like that. I'm staying out of that for now.
Change-Id: I81b50c39cd6e908c4c95651829b679425de87a28
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Provide a sane means of adding the -Werror compiler flag.
Currently, some of our jenkins.sh add -Werror by passing 'CFLAGS="-Werror"',
but that actually *overwrites* all the other CFLAGS we might want to have set.
Maintain these exceptions from -Werror:
a) deprecation (allow upstream to mark deprecation without breaking builds);
b) "#warning" pragmas (allow to remind ourselves of errors without breaking
builds)
As a last configure step before generating the output files, print the complete
CFLAGS and CPPFLAGS by means of AC_MSG_RESULT.
Change-Id: Ic5c8e68b64cd890b3309b4b26c7f22bde1edba83
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|