On 13 December 2014 at 21:20, Bill Fischofer <[email protected]> wrote: > Background: > > In the ODP linux-generic implementation we're currently defining ODP > abstract types like this: > > typedef uint32_t odp_buffer_t; > typedef uint32_t odp_buffer_pool_t; > typedef uint32_t odp_packet_t; > ...etc. > > While this provides documentation, unfortunately typedefs in C are weak, > meaning that although odp_buffer_t and odp_buffer_pool_t may look different, > C will treat them as equivalent since they are just aliases for uint32_t. > This was brought home in a recent bug we detected in odp_crypto.c. > > As part of the API normalization work for ODP v1.0 the syntax and semantics > of odp_packet_copy() were changed from: > > int odp_packet_copy(odp_packet_t pkt_dst, odp_packet_t pkt_src); > > to > > odp_packet_t odp_packet_copy(odp_packet_t pkt, odp_buffer_pool_t pool); > > However, odp_crypto.c contained the line: > > odp_packet_copy(params->out_pkt, params->pkt); > > which C happily accepted without comment even though it's obviously very > wrong under the new definition of the API. With strong typing, this would > be flagged at compile time, which is clearly preferable to letting things > like this slip by. > > Strong typing in C: > > There are a number of techniques to achieve strong typing in C, but the most > straightforward way to make typedefs strong is by changing the type > definitions to the following: > > #define odp_handle_t struct { uint32_t val; } This might have performance implications. Often structs are passed by reference (a pointer to a temporary struct) and not by value even when the struct is so small it could easily have fitted into registers. Same for returning structs. The calling convention (ABI) of the target decides.
> > typedef odp_handle_t odp_buffer_t; > typedef odp_handle_t odp_buffer_pool_t; > typedef odp_handle_t odp_packet_t; > ...etc. > > Under these type definitions you cannot mix and match ODP types without some > sort of explicit type conversion, which is why the API provides routines > like odp_packet_to_buffer() and odp_packet_from_buffer(). Moreover, this > change is transparent for the most part to existing correct ODP > applications, so that's all good. > > The only downside is that with weak types today you can write code like: > > if (buf == ODP_BUFFER_INVALID) ... > > to check things like the return value from odp_buffer_alloc(). However with > strong types C will reject that since operators like == require numeric > operands and unlike C++, C does not support operator overloading. This is > easily remedied by including a simple macro: > > #define odp_equal(x, y) (x.val == y.val) > > in odp_platform_types.h so that the application can write: > > if (odp_equal(buf, ODP_BUFFER_INVALID)) ... You need another macro for comparing the value of some ODP handle with a constant value like ODP_BUFFER_INVALID (unless ODP_BUFFER_INVALID itself is a handle of the struct kind). > > instead. > > However this does require a source code change on the part of the > application, hence this RFC. I believe this is a very small price to pay for > the improved type safety and would like to propose that we switch to strong > typing prior to the official ODP v1.0 release, which would mean officially > adding and documenting odp_equal() as an ODP API. > > Comments, please. I think this is a problem best solved by other tool like sparse, not by the C compiler. Or if we think the compiler itself should do these types of checks (need a good argument for that), then we should migrate to C++. > > > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp > _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
