5 Apr
2016
5 Apr
'16
11:08 p.m.
On 04 Apr 2016, at 16:06, msuraev(a)sysmocom.de wrote:
@@ -1321,8 +1321,8 @@ static int ipaccess_bsc_read_cb(struct osmo_fd *bfd)
bsc->cfg ? bsc->cfg->nr : -1);
else
LOGP(DNAT, LOGL_ERROR,
- "Stream error on BSC Nr: %d. Failed to parse ip access message:
%d\n",
- bsc->cfg ? bsc->cfg->nr : -1, ret);
+ "Stream error on BSC Nr: %d. Failed to parse ip access message: %d
(%s)\n",
+ bsc->cfg ? bsc->cfg->nr : -1, ret, strerror(-ret));
good! but separate patch. And have a look at where ret is coming from that you need to
invert it.
bsc_replace_string(_nat, &_nat->include_file, argv[0]);
+
+ llist_for_each_entry_safe(con1, con2, &_nat->bsc_connections,
+ list_entry) {
+ if (con1->cfg)
+ if (con1->cfg->token_updated || con1->cfg->remove)
+ bsc_close_connection(con1);
+ }
In terms of style the following is preferred as it is reducing the level of indentation
and makes you see what the code is about (instead of shifting it to the right)
if (!con1->cfg)
continue;
if (!con->cfg->token_updated && !con1->cfg->remove)
continue;
bsc_close_connection(con1);
resulting assembly should not be any different and the ultimate action is seen.
+
+ llist_for_each_entry_safe(cf1, cf2, &_nat->bsc_configs, entry) {
+ if (cf1->remove) {
+ bsc_config_free(cf1);
+ _nat->num_bsc--;
that one is a "nasty" topic
1.) this should be within bsc_config_free
2.)
bsc_config_alloc(..) {
conf->nr = nat->num_bsc;
and the code assumes that conf->nr is unique so if we do num_bsc-- and then num_bsc++
again we might re-use an existing id and break the assumption..
What I propose (and in a separate patch)
* Remove the requirement to assign BSCs in order. I thought I had lifted it a long time
ago (but I did not)
* Change bsc_config_alloc to include the nr to be assigned
* change cfg_bsc_mcd to first search the given BSC and then create one..
holger