Hi, On 01/07/2011 04:54 PM, Diego Biurrun wrote:
On Wed, Jan 05, 2011 at 08:04:14AM +0200, Miika Komu wrote:On 04/01/11 20:06, Diego Biurrun wrote:I removed most, but not yet all, typedefs and _t namespace pollution in HIPL. Some of the remaining ones can possibly be kept, but of course we have to rename them to address the namespace pollution. The remaining typedefs can be grouped into a few classes: lib/core/hashtable.h:typedef DECLARE_LHASH_OF(HIP_HT) HIP_HASHTABLE; lib/core/hashtable.h:typedef LHASH_OF(HIP_HT) HIP_HASHTABLE_TYPE; lib/core/hashtable.h:typedef LHASH HIP_HASHTABLE; lib/core/hashtable.h:typedef HIP_HASHTABLE HIP_HASHTABLE_TYPE; These will go away if/when we make OpenSSL 1.0 a requirement and remove these backwards compatibility hacks.I guess we have to wait for CentOS. What about OpenWRT? Does it have OpenSSL 1.0.0? To avoid forgetting, I took the liberty to file this to bugzilla: https://bugs.launchpad.net/hipl/+bug/697522lib/core/list.h:typedef LHASH_NODE hip_list_t; This one is suspicious, I see no reason to hide the actual type that is being used.I guess the original purpose was to minimize the impact to the code. You can change this if you want to, but we have indirection (wrappers) also in the functions that access the actual list structures.I dropped the typedef. What do you mean by indirection/wrappers? I don't see any...
we're operating the lists indirectly with wrappers such as list_add() instead of lh_insert(). Thus, it would have made sense to have wrapper also for the actual structures. Not a big issue, though.
lib/core/prefix.h:typedef uint32_t hip_closest_prefix_type; lib/core/protodefs.h:typedef uint8_t hip_hdr_type_t; lib/core/protodefs.h:typedef uint8_t hip_hdr_len_t; lib/core/protodefs.h:typedef uint16_t se_family_t; lib/core/protodefs.h:typedef uint16_t se_length_t; lib/core/protodefs.h:typedef uint16_t se_hip_flags_t; lib/core/protodefs.h:typedef uint16_t hip_hdr_err_t; lib/core/protodefs.h:typedef uint16_t hip_tlv_type_t; lib/core/protodefs.h:typedef uint16_t hip_tlv_len_t; lib/core/protodefs.h:typedef uint16_t hip_transform_suite_t; lib/core/protodefs.h:typedef uint16_t hip_controls_t; lib/core/protodefs.h:typedef uint32_t sa_eid_t; At least some of these look useful, but possibly some of them can be merged into one class. We need new names for all of them.These are here for protocol change management; if the size would change in the specification, the change would consist of a single line. What about changing _t to something else like _td to avoid posix namespace pollution?I think hip_closest_prefix type shows that no suffix is necessary to have a sensible name for the type. I propose the following names, which drop the _t and _type suffixes: typedef uint32_t hip_closest_prefix_type; typedef uint8_t hip_hdr; typedef uint8_t hip_hdr_len; typedef uint16_t se_family; typedef uint16_t se_length; typedef uint16_t se_hip_flags; typedef uint16_t hip_hdr_err; typedef uint16_t hip_tlv; typedef uint16_t hip_tlv_len; typedef uint16_t hip_transform_suite; typedef uint16_t hip_controls; typedef uint32_t sa_eid;
Fine.
lib/core/protodefs.h:typedef struct in6_addr hip_hit_t; lib/core/protodefs.h:typedef struct in_addr hip_lsi_t; hipd/oppipdb.h:typedef struct in6_addr hip_oppip_t; I'm unsure about these, but I'm tempted to think we should just drop them. Am I missing a case where a HIT is not an IPv6 address?It's there to make a semantic difference between a HIT and an IPv6 address: https://bugs.launchpad.net/hipl/+bug/648684 My comment about _t -> _td applies here too.Just dropping the _t suffix should work fine here as well.
Ok.
