Maxim had an interesting sugestion I will investigate for a free tool http://www.cs.umd.edu/~jfoster/cqual/
On 15 December 2014 at 08:25, Bill Fischofer <[email protected]> wrote: > > We've said an implementation is free to define ODP abstract types however > it wishes. If I were an implementer, then defining these handles strongly > as shown here would be legitimate by what we've said, however it would not > work since the current API assumes that == can be used on handles > directly. This suggests that we *should* be using odp_equal() rather > than == independent of how we choose to implement these typedefs ourselves > in linux-generic. > > > On Mon, Dec 15, 2014 at 7:18 AM, Ola Liljedahl <[email protected]> > wrote: >> >> On 15 December 2014 at 14:02, Bill Fischofer <[email protected]> >> wrote: >> > I have a proof-of-concept implementation of this idea available at >> > http://git.linaro.org/people/bill.fischofer/odp_strongtypes.git and it >> > doesn't appear to suffer any performance degradation compared to the >> weak >> > type version. >> > >> > If we can get sparse or some other tool to do this I'm fine with that >> > approach, but this issue needs to be addressed by some means. I think >> it's >> > simpler if the C compiler can do it by itself rather than requiring some >> > other tool that may not be used or used with any regularity. >> > >> > The other issue is just how abstract are the ODP abstract types? We've >> > hand-waved this issue but as currently written the code will fail if >> someone >> > tries to define handles as anything other than 32-bit quantities. >> I expect some more performance and hardware oriented ODP >> implementations would use pointers for e.g. the buffer derived types. >> >> To what extent does this strict type checking affect other ODP >> implementations from using scalar or pointer types for handles? An >> implementation can always provide its own type definitions but must >> support the additional functions we defined (e.g. compare or is_equal >> functions). >> >> > >> > On Mon, Dec 15, 2014 at 6:03 AM, Ola Liljedahl < >> [email protected]> >> > wrote: >> >> >> >> 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 >> > -- *Mike Holmes* Linaro Sr Technical Manager LNG - ODP
_______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
