| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Callers require to know whether the returned ERROR cmd was received or
generated locally, in order to send it or do something with it locally.
Related: OS#3394
Change-Id: Ide9170e5c31967c353f8fe4e8227e64130b91eae
|
|
Our implementation generates ERROR CTRL messages with ID=error when it
is unable to parse a CMD. However, it doesn't account for them when
trying to receive and parse this kind of message. As a result, it will
return an ERROR message with a different description. This commit fixes
the old behaviour to success at parsing and returning the received
description to the caller.
Change-Id: I564ab1a7e845388f87accda44fbf165e5adc2480
|
|
Change-Id: Ia890d4b841ef02342cc1cf7f5926866b040dc8ab
|
|
Closes: OS#3293
Change-Id: I8dc2f24d4bf557ff7bb0f2f46881f9f8d9d7f86f
|
|
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
|
|
So far, error reporting just says "Trap/Reply", more accurately report 'GET
REPLY', 'SET REPLY' and 'TRAP' as appropriate.
Change-Id: Ic25a251502499aeda4e2952ec4190a1fa0bebb01
|
|
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
|
|
If a control command fails to parse, we so far discard specific error messages
and instead send just "Command parser error".
In ctrl_cmd_parse() we actually compose detailed error replies, but in the end
simply talloc_free() them and return NULL.
A first step to report these errors to the ctrl command issuer is to not return
NULL and instead return the cmd with type = CTRL_TYPE_ERROR. Add
ctrl_cmd_parse2() to return such instead of NULL.
To stay API compatible, provide ctrl_cmd_parse2() to return a cmd on errors.
ctrl_cmd_parse() retains identical behavior but becomes just a simple wrapper
around ctrl_cmd_parse2() which discards the cmd on error.
No need really to deprecate ctrl_cmd_parse() yet; especially as long as
compiler warnings might break jenkins builds.
Change-Id: I5047c9f977d70b03eea77cbcfd2b96d43ea46880
|
|
Let's fix some erroneous/accidential references to wrong license,
update copyright information where applicable and introduce a
SPDX-License-Identifier to all files.
Change-Id: I39af26c6aaaf5c926966391f6565fc5936be21af
|
|
Fixes following warning:
control_cmd.c:294:16: warning: comparison of unsigned enum expression < 0 is always false [-Wtautological-compare]
if (cmd->type < 0 || cmd->type == CTRL_TYPE_UNKNOWN) {
Change-Id: I3df8a4f646222337927d9e3cac6d09a8a05cb20c
|
|
Considering the various styles and implications found in the sources, edit
scores of files to follow the same API doc guidelines around the doxygen
grouping and the \file tag.
Many files now show a short description in the generated API doc that was so
far only available as C comment.
The guidelines and reasoning behind it is documented at
https://osmocom.org/projects/cellular-infrastructure/wiki/Guidelines_for_API_documentation
In some instances, remove file comments and add to the corresponding group
instead, to be shared among several files (e.g. bitvec).
Change-Id: Ifa70e77e90462b5eb2b0457c70fd25275910c72b
|
|
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
|
|
In ctrl_cmd_parse(), fix missing check for not parseable ctrl type.
Fixup for Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28.
Change-Id: I7f8055225e3ee04b2a723bae07b12c42618963a0
|
|
Use value_string for enum ctrl_type instead of custom code. Add
corresponding unit tests.
Related: OS#1615
Change-Id: Icd4e96dd9f00876cb70b43cfcf42ab4f10311b28
|
|
The 'strtok_r' function requires passing a NULL as the first parameter
on subsequent calls in order to ensure the code picks up where it left
off on a previous call. However, Coverity doesn't quite realize this
and points out that if a NULL was passed in as the third argument it
would result in a possible NULL deref because the strtok_r function will
assign the third argument to the first in the call is NULL.
Change-Id: I7a9d08d0d4eae76a5207d285e32a25d1b384a57f
Fixes: Coverity CID 135186
|
|
We check that the amount of commands is not more than
we have but we don't check it the other way. It appears
that the vector is allowed to be bigger than the amount
of commands. So we match a prefix of a longer command
depending on the installation order.
|
|
Sometimes a control interface command cannot be processed
and responded immediately, but we need to process it asynchronously.
In order to support this, we introduce the 'ctrl_cmd_def', which
represents such a deferred command. It is created by the service
implementing the command using ctrl_cmd_def_make(), and a response is
later sent using ctrl_cmd_def_send().
ctrl_cmd_def_is_zombie() must be called to handle the case where
the control connection has disconnected/died between receiving the
command and sending the response.
|
|
|
|
... and make libctrl code use it
|
|
|
|
|
