On 15 December 2014 at 06:26, Ola Liljedahl <[email protected]> wrote: > > 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++. >
The gimple lint tool is capable of distinguishing the weak types, did anyone try passing this issue through coverity to see if it is capable of making the distinction ? I'd rather find a tool than create unusual C code. > > > > > > > > > _______________________________________________ > > 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 > -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
