[PATCH] NAT: allow allocating BSC in arbitrary order

Neels Hofmeyr nhofmeyr at sysmocom.de
Wed Apr 6 23:04:50 UTC 2016


>  
>  	conf->token = talloc_strdup(conf, token);
> -	conf->nr = nat->num_bsc;
> +	conf->nr = number;

I think you could completely remove the num_bsc variable? It looks like its
sole use was to determine the next available BSC number without iterating the
list.

>  	conf->nat = nat;
>  	conf->max_endpoints = 32;
>  	conf->paging_group = PAGIN_GROUP_UNASSIGNED;
> @@ -205,6 +206,10 @@ void bsc_config_free(struct bsc_config *cfg)
>  	llist_del(&cfg->entry);
>  	rate_ctr_group_free(cfg->stats.ctrg);
>  	talloc_free(cfg);
> +	cfg->nat->num_bsc--;
> +	if (cfg->nat->num_bsc < 0)
> +		LOGP(DNAT, LOGL_ERROR, "Internal error while deallocating BSC "
> +		     "config: negative BSC index!\n");
>  }

I don't understand why you would add this check for negative BSC index.
The llist_del() should ensure that we don't double free a BSC, right?

Also nice would be to add a test case that has a non-null BSC number, to show
that having gaps in the numbering doesn't have side effects.

~Neels
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20160407/8af27940/attachment.bin>


More information about the OpenBSC mailing list