From: Pablo Neira Ayuso pablo@gnumonks.org
This patch is a RFC, I can add the prefix osmocom_ to all functions in libosmocore to fix with the existing namespace pollution.
This task was proposed by Harald.
Let me know if you are OK with this approach and I'll send a patchset along this week. --- include/osmocom/core/backtrace.h | 2 +- src/backtrace.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/osmocom/core/backtrace.h b/include/osmocom/core/backtrace.h index bbbb2c2..7c248aa 100644 --- a/include/osmocom/core/backtrace.h +++ b/include/osmocom/core/backtrace.h @@ -1,6 +1,6 @@ #ifndef _OSMO_BACKTRACE_H_ #define _OSMO_BACKTRACE_H_
-void generate_backtrace(); +void osmocom_generate_backtrace();
#endif diff --git a/src/backtrace.c b/src/backtrace.c index ecd6b9c..5c609bb 100644 --- a/src/backtrace.c +++ b/src/backtrace.c @@ -29,7 +29,7 @@
#ifdef HAVE_EXECINFO_H #include <execinfo.h> -void generate_backtrace() +void osmocom_generate_backtrace() { int i, nptrs; void *buffer[100];
Hi,
This patch is a RFC, I can add the prefix osmocom_ to all functions in libosmocore to fix with the existing namespace pollution.
Well, use osmo_ not osmocom_ (that's what's used at several places already).
Second, I wouldn't say _all_ ... I mean when there is already a prefix, I see no point adding another (things like msgb_, vty_, bitvec_, log_, gsmtap_, rsl_, gsm48_ ...).
For things that are too generic, like 'generate_backtrace' or 'plugin_load_all', then sure.
Sylvain
Hi Pablo,
On Sun, Mar 27, 2011 at 09:37:54PM +0200, pablo@gnumonks.org wrote:
This patch is a RFC, I can add the prefix osmocom_ to all functions in libosmocore to fix with the existing namespace pollution.
I think 'osmocom_' is too long. 'osmo_' should be better. And yes, generally it would make sense to prefix most functions.
Exceptions: * libosmovty should have a common vty_ prefix, but not osmo. * for libosmovty we should probably simply limit the number of exported symbols. There is e.g. all the buffer_*() and vector_*() use internally, but there is no need to globally export all those symbols. * no osmo_ prefix for msgb_* and tlv_*, as they are used everywhere * no prefix for talloc * bitvec_* -> osmo_bv_* * bsc_*_timer -> osmo_timer_* * *_signal_* -> osmo_signal_*
Let me know if you are OK with this approach and I'll send a patchset along this week.
I think it's great. Once the namespace is cleaned up, I think we can head for a 1.0 release of the libraries.
Hi!
On 27/03/11 22:20, Harald Welte wrote:
Hi Pablo,
On Sun, Mar 27, 2011 at 09:37:54PM +0200, pablo@gnumonks.org wrote:
This patch is a RFC, I can add the prefix osmocom_ to all functions in libosmocore to fix with the existing namespace pollution.
I think 'osmocom_' is too long. 'osmo_' should be better. And yes, generally it would make sense to prefix most functions.
Exceptions:
- libosmovty should have a common vty_ prefix, but not osmo.
- for libosmovty we should probably simply limit the number of exported symbols. There is e.g. all the buffer_*() and vector_*() use internally, but there is no need to globally export all those symbols.
- no osmo_ prefix for msgb_* and tlv_*, as they are used everywhere
- no prefix for talloc
- bitvec_* -> osmo_bv_*
- bsc_*_timer -> osmo_timer_*
- *_signal_* -> osmo_signal_*
thanks for the feedback!
Let me know if you are OK with this approach and I'll send a patchset along this week.
I think it's great. Once the namespace is cleaned up, I think we can head for a 1.0 release of the libraries.
those are good news.
BTW, do you plan to maintain a stable ABI along releases for libraries?
Another proposition: it may be a good idea to use some EXPORT_SYMBOL() macro, similar to what we use in the Linux kernel, to explicit tell what symbols of the library are exported.
We use this in libmnl: http://git.netfilter.org/cgi-bin/gitweb.cgi?p=libmnl.git;a=summary
Hi Pablo,
On Mon, Mar 28, 2011 at 04:40:08PM +0200, Pablo Neira Ayuso wrote:
Let me know if you are OK with this approach and I'll send a patchset along this week.
I think it's great. Once the namespace is cleaned up, I think we can head for a 1.0 release of the libraries.
those are good news.
BTW, do you plan to maintain a stable ABI along releases for libraries?
that's the idea once we have the namespace pollution fixed and release a 1.0
Another proposition: it may be a good idea to use some EXPORT_SYMBOL() macro, similar to what we use in the Linux kernel, to explicit tell what symbols of the library are exported.
I think it makes sense, indeed. Feel free to work on a patch, after the namespace conversion has been done. Also, don't forget the wireshark dissector related work (which I think is more important).
Thanks, Harald
Some minor questions:
On 27/03/11 22:20, Harald Welte wrote:
Exceptions:
- libosmovty should have a common vty_ prefix, but not osmo.
- for libosmovty we should probably simply limit the number of exported symbols. There is e.g. all the buffer_*() and vector_*() use internally, but there is no need to globally export all those symbols.
- no osmo_ prefix for msgb_* and tlv_*, as they are used everywhere
- no prefix for talloc
- bitvec_* -> osmo_bv_*
enum bit_value -> enum osmo_bit_value struct bitvec -> struct osmo_bitvec
- bsc_*_timer -> osmo_timer_*
there are some inconsistencies in the API naming in the timer bits:
bsc_add_timer(...) bsc_timer_pending(...)
I can put the timer postfix in the end, so it looks consistent.
struct timer_list should be struct osmo_timer_list, right?
- *_signal_* -> osmo_signal_*
Is there any plan to remove the static lists inside the library?
I can make this in follow up patches. This would require more testing since it's not a simple renaming.
Hi Pablo,
On Mon, Mar 28, 2011 at 05:27:42PM +0200, Pablo Neira Ayuso wrote:
Some minor questions:
On 27/03/11 22:20, Harald Welte wrote:
Exceptions:
- libosmovty should have a common vty_ prefix, but not osmo.
- for libosmovty we should probably simply limit the number of exported symbols. There is e.g. all the buffer_*() and vector_*() use internally, but there is no need to globally export all those symbols.
- no osmo_ prefix for msgb_* and tlv_*, as they are used everywhere
- no prefix for talloc
- bitvec_* -> osmo_bv_*
enum bit_value -> enum osmo_bit_value struct bitvec -> struct osmo_bitvec
both fine with me. I think Sylvain wanted to be a bit less 'aggressive' with renaming, but the point is: We're breaking the API anyway, so we might as well do it properly once and hopefully not have to care about it in the future.
there are some inconsistencies in the API naming in the timer bits:
bsc_add_timer(...) bsc_timer_pending(...)
I can put the timer postfix in the end, so it looks consistent.
I think it should all be called osmo_timer_* for consistency.
struct timer_list should be struct osmo_timer_list, right?
ACK.
- *_signal_* -> osmo_signal_*
Is there any plan to remove the static lists inside the library?
no, I don't have such plans, and I don't think it is neccessarry. The signalling system is not intended to have multiple instances. There is one in every process, that's it.
Regards, Harald
-
246tnt＠gmail.com -
Harald Welte -
Pablo Neira Ayuso -
pablo＠gnumonks.org