Hi Neels,

On Tue, Jan 22, 2019 at 03:00:31AM +0100, Neels Hofmeyr wrote:
> I think we all agree that what happened with the msgb_wrap_with_TL() is a 
> prime
> example about how absolutely *not* to do things.
> This makes me want to rewrite libosmocore history.

I had the same feeling yesterday.  To clarify "rewrite libosmocore git history" 
is
probably what you hinted here :)

> Let's try to avoid this kind of series of events in the future.
> 
> * Separate function definitions must not have identical names.
>   * This applies both to the master HEADs as well as throughout entire API 
> history.
>   * Especially functions moved to another source tree *must* change their 
> name.

This is generally what we do.  And "generally", we prefix them with osmo_ in the
libraries, and don't permit the osmo_ prefix for symbols in applications.  
However,
the msgb_ code pre-dates the osmo_ prefix - it even predates the name Osmocom as
the project name.

So we have some legacy prefixes that are "reserved" for use in libosmo*. Those 
are

bitvec_
gsmtap_
log_
msgb_
rate_ctr_
abis_nm_
gprs_cipher_
gsm0341_
gsm0480_
gsm0502_
gsm0503_
gsm0808_
gsm29118_
gsm0858_
gsm340_
gsm411_
gsm414_
gsm48_
gsm610_
gsm620_
gsm690_
gsm_7bit_
lapd_
lapdm_
milenage_
rsl_
rxlev_
tlv_
ipa_ccm
bssgp_
gprs_ns_
gprs_nsvc_
btsctx_
osim_
vty_
vector_
telnet_
config_
cmd_
buffer_

Unfortunately a lot of them are rather generic, so it's hard to avoid, at least 
in absence
of any automatic tests for it.  It's a separate discussion whether we should 
e.g. stop to
export los of libosmovty internals (buffer, vector, cmd) - and if we should 
prefix all
symbols in libosmo* with osmo_* providing the older names only as backwards 
compatibility
layer with possibly weak symbols to be able to migrate to a cleaner namespace 
at least at
some point in the future.

Furthermore, there is one factors that made this particular instance even more 
problematic:

The respective functions were inline functions.  Otherwise we could have simply 
turned
the library symbol into a weak symbol, making any application implementation 
supersede
the library one.

> * Such API changes in libosmocore should be tested with "all" depending 
> programs.

ACK.  So what's needed is some kind of build job that continuously builds old 
applications
against modern libosmo*.  This could be run at least once per day/night, so we 
get notified
if we introduce any such breakage and can fix libosmo* shortly to keep the 
incompatiblity
something that appeared for a few days in the history, at maximum.

> * API fixes in libosmocore should ideally build with both past and future
>   versions of depending source trees, i.e. should not need a matching patch in
>   the other source trees.

That goes without saying.  It just wasn't possible in this case.

Regards,
        Harald
-- 
- Harald Welte <[email protected]>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)

Reply via email to