On 15 December 2014 at 12:51, Mike Holmes <[email protected]> wrote: > > > 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. Perhaps splint. It has some kind of support for "abstract" types. The basic idea seems to be to prevent users of abstract types to access implementation-specific details. I don't know if it detects the mixing if different abstract types but strong type checking is one feature of splint.
> >> >> >> > >> > >> > >> > _______________________________________________ >> > 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
