| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
| |
Change-Id: I3c8e95aaa1ca222d4cd1395e548f8461bf9d4cd6
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Validate that incoming CTRL commands...
- have decimal IDs,
- return error on trailing characters,
- have invalid characters in variable identifiers,
- send detailed error messages as reply to the requestor.
Adjust ctrl_test.{c,ok}, which best show the change in behavior.
Message handling causes log messages on stderr; previously, stderr was empty.
Add '[ignore]' in testsuite.at so that the nonempty stderr doesn't cause test
failures.
Change-Id: I96a9b6b6a3a5e0b80513aa9eaa727ae8c9c7d7a1
|
|
|
|
| |
Change-Id: Id11bc326be2f0bc2746a928354e416495a18baf7
|
|
|
|
|
|
|
|
|
|
|
|
| |
In ctrl_handle_msg() (code recently propagated from handle_control_read()),
talloc_free() the parsed ctrl_cmd in all code paths. In particular, a free was
missing in case ctrl_cmd_handle() returns CTRL_CMD_HANDLED.
CTRL_CMD_HANDLED is triggered by GET_REPLY / SET_REPLY parsing, as show by
ctrl_test.c. With the memleak fixed, adjust expected test output and make a
detected mem leak abort the test immediately.
Change-Id: Id583b413f8b8bd16e5cf92a8a9e8663903646381
|
|
|
|
|
|
|
| |
The "memleak!" output shows messages that lack a talloc_free() of the parsed
ctrl command buffer. The leak shall be fixed in a subsequent patch.
Change-Id: I2c3e4d08b769b9cd77593362ea36a28d681cd042
|
|
|
|
|
|
|
| |
Subsequent patches that tighten CTRL input validation will make the results
more interesting.
Change-Id: Idd4cc7d193db1a7d761f72ed33ed46eea450a78f
|
|
|
|
| |
Change-Id: I12d3828dcc925f97fde11c360f1d60f3bd8cad8b
|
|
|
|
|
|
|
|
| |
This uncovers some interesting behavior of the CTRL interface which we may want
to guard against in subsequent patches: trailing whitespace, ignored tokens,
special characters as cmd->id.
Change-Id: If7af06d50ca71fd528b08cd70310774d5a53f0f7
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Don't use CTRL_TYPE_UNKNOWN as value_string[] terminator, use an explicit, more
obvious { 0, NULL } termination. Set an explicit string for CTRL_TYPE_UNKNOWN.
No other value_string[]s to date have such a "hidden" terminator.
BTW, a { 0, "string" } item is not a terminator, only { 0, NULL } is, so we can
set a string for CTRL_TYPE_UNKNOWN == 0.
Also, having a string value for CTRL_TYPE_UNKNOWN is not harmful because all
code paths explicitly check for the CTRL_TYPE_*s that are valid.
Adjust the test expectation.
From the ctrl_type_vals enum, remove the = 0, because it is implicitly 0
anyway.
One motivation to press this fixup: I am trying to add a script that checks
whether all value_string[]s are terminated to our jenkins jobs, and to find
that this one is terminated, it would need to interpret the CTRL_TYPE_UNKNOWN
constant, which would make things far more complex. At this point, all of the
value_string[]s have an explicit termination, and I would like to enforce this
from now on -- for readable code and to not spend more time on the validator.
The patch adding ctrl_type_vals (Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28) was
accepted by another reviewer before I could reconfirm my -1, so this is a fixup
to enable the termination checking script patches.
Related: I2bc93ab4781487e7685cfb63091a489cd126b1a8 (adds script to libosmocore)
I7fe3678b524d602fc6aa14bc0ed06308df809a3e (uses in jenkins.sh)
Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28 (adds ctrl_type_vals)
Change-Id: Ia99f37464c7b36b587da2cc78f52c82725f02cbc
|
|
Use value_string for enum ctrl_type instead of custom code. Add
corresponding unit tests.
Related: OS#1615
Change-Id: Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28
|