Public API is not convenient

Bug #799200 reported by Didier Barvaux
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
rohc
Status tracked in Rohc-main
Rohc-main
Fix Released
Wishlist
Didier Barvaux

Bug Description

The current public API is difficult to use:
  1. several functions shall be called to initiate the library,
  2. many functions do not report success or error,
  3. many functions mix status and packet length in return value,
  4. there are 2 functions for decompression (one for small CIDs and one for
     large/small CIDs),
  5. some functions do not use the "rohc_" prefix in their names,
  6. some functions related to statistics store their output in strings,
  7. there is no mean to configure the way the library logs infos,
  8. every user parameters shall be set by a specific function (one function
     for all is not convenient and not future-proof),
  9. many private functions/structs/constants are exported and may be used by
     users while they should not,
 10. merge the 3 librohc_(common|comp|decomp).so into one single library
     librohc.so
 11. put all public headers in a subdir, ie. /usr/include/rohc/*.h
 12. make configuration uni- and bi-directionnal mode clearer
 13. and so on...

Tags: api library
Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

The following blog post contains some advices that might be followed during API rewrite:
http://davidz25.blogspot.com/2011/06/writing-c-library-part-1.html

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Check if the visibility attribute might be used: http://gcc.gnu.org/wiki/Visibility

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

A blog post about symbol visibility: http://blog.flameeyes.eu/2011/01/20/hide-those-symbols

tags: added: library
Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

A blog post about writing a great public API: http://davidz25.blogspot.com/2011/07/writing-c-library-part-5.html

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Be careful if possible not to remove the old public API while adding the new one. The old API will be removed in a later release (2.0.0 ?).

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Use the "deprecated (msg)" function attribute to declare the old public API deprecated. See http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html for more details about the "deprecated" attribute.

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :
description: updated
description: updated
Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Replace the rohc_activate_profile() function by:
  bool rohc_comp_enable_profile(struct rohc_comp *const comp, const unsigned int profile);
  bool rohc_comp_disable_profile(struct rohc_comp *const comp, const unsigned int profile);

And the corresponding variadic functions:
  bool rohc_comp_enable_profiles(struct rohc_comp *const comp, ...);
  bool rohc_comp_disable_profiles(struct rohc_comp *const comp, ...);

See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/766

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Add similar functions for decompressor:
  bool rohc_decomp_enable_profile(struct rohc_decomp *const decomp, const unsigned int profile);
  bool rohc_decomp_disable_profile(struct rohc_decomp *const decomp, const unsigned int profile);
  bool rohc_decomp_enable_profiles(struct rohc_decomp *const decomp, ...);
  bool rohc_decomp_disable_profiles(struct rohc_decomp *const decomp, ...);

See http://bazaar.launchpad.net/~didier-barvaux/rohc/main/revision/769

description: updated
Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

rev 845: Define new type rohc_cid_t for CID and MAX_CID.
rev 846: API: add rohc_comp_new() and rohc_comp_free().
rev 848: Replace rohc_mode type by rohc_mode_t.
rev 849: API: add rohc_decomp_new() and rohc_decomp_free().
rev 856: API: add new rohc_decomp_get_max_cid() function.
rev 857: API: add new rohc_decomp_get_cid_type() function.
rev 858: API: deprecate rohc_decomp_set_cid_type() and rohc_decomp_set_max_cid().
rev 859: API: deprecate rohc_c_set_large_cid() and rohc_c_set_max_cid().
rev 860: API: add new rohc_decomp_get_mrru() function.
rev 861: API: deprecate rohc_c_using_small_cid()
rev 862: API: deprecate rohc_c_set_header()
rev 864: API: deprecate rohc_c_set_enable() and rohc_c_is_enabled()
rev 869: API: install public headers in the rohc/ subdirectory
rev 870: API: add rohc/ prefix for includes in public headers.
rev 871: API: build an additional librohc.so to aggregate librohc_*.so

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

TODO:
 - replace then deprecate user_interactions()
 - rohc_status_t + rohc_get_status_descr()
 - rohc_comp_compress / rohc_decomp_decompress
 - hide symbols (https://blog.flameeyes.eu/2011/01/hide-those-symbols)
 - update wiki about deprecation (http://rohc-lib.org/wiki/doku.php?id=library-migration#api_changes_between_16x_and_17x)

Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

rev 974: API: use ROHC_ prefix for names of packets and extensions.
rev 975: API: replace enum rohc_c_state by enum rohc_comp_state_t
rev 976: API: replace enum rohc_d_state by enum rohc_decomp_state_t
rev 977: API: deprecate function clear_statistics().
rev 879: Be compatible with C++ applications.
rev 880: Avoid using variables or functions named 'index'.
rev 881: API: introduce rohc_profile_t for the different IDs of ROHC profiles
rev 882: API: deprecate rohc_d_statistics().
rev 885: API: avoid including stdio.h if deprecated API is not included in build.
rev 888: Fix distcheck after change at rev 870.
rev 890: Fix wrong assertion added at revision 882.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.