On 4 February 2015 at 13:28, Savolainen, Petri (NSN - FI/Espoo) <petri.savolai...@nsn.com> wrote: > > >> -----Original Message----- >> From: ext Ola Liljedahl [mailto:ola.liljed...@linaro.org] >> Sent: Wednesday, February 04, 2015 12:01 PM >> To: Savolainen, Petri (NSN - FI/Espoo) >> Cc: lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] [PATCHv5 05/18] api: odp_buffer.h: undefined >> behavior description >> >> On 4 February 2015 at 10:51, Savolainen, Petri (NSN - FI/Espoo) >> <petri.savolai...@nsn.com> wrote: >> > >> > >> >> -----Original Message----- >> >> From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp- >> >> boun...@lists.linaro.org] On Behalf Of ext Ola Liljedahl >> >> Sent: Tuesday, February 03, 2015 6:48 PM >> >> To: lng-odp@lists.linaro.org >> >> Subject: [lng-odp] [PATCHv5 05/18] api: odp_buffer.h: undefined >> behavior >> >> description >> >> >> >> Documented API calls which are guaranteed to handle invalid/stale >> handles. >> >> >> >> Signed-off-by: Ola Liljedahl <ola.liljed...@linaro.org> >> >> --- >> >> (This document/code contribution attached is provided under the terms >> of >> >> agreement LES-LTM-21309) >> >> >> >> include/odp/api/buffer.h | 4 +++- >> >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/include/odp/api/buffer.h b/include/odp/api/buffer.h >> >> index 12b2f5a..1eb2e5a 100644 >> >> --- a/include/odp/api/buffer.h >> >> +++ b/include/odp/api/buffer.h >> >> @@ -88,7 +88,9 @@ uint32_t odp_buffer_size(odp_buffer_t buf); >> >> /** >> >> * Tests if buffer is valid >> >> * >> >> - * @param buf Buffer handle >> >> + * @param buf Buffer handle (possibly invalid) >> >> + * @note This is the only buffer API function which accepts invalid >> >> buffer >> >> + * handles (any bit value) without causing undefined behavior. >> >> * >> >> * @retval 1 Buffer handle represents a valid buffer. >> >> * @retval 0 Buffer handle does not represent a valid buffer. >> >> -- >> > >> > Originally, this and odp_packet_is_valid() were defined to enable full >> sanity check over the object. During debugging user could insert these on >> the way and (hopefully) catch the point where e.g. packet metadata (e.g. >> segment linking) was corrupted. >> Should we specify this "full sanity check of object" as well? That >> part is not obvious now and an ODP implementer could easily interpret >> "represents a valid buffer" in a less complete way. >> Perhaps this function needs more return values. E.g. >> @retval 0 Buffer handle is invalid (no further checks done) >> @retval 1 Buffer handle is valid but buffer is corrupted >> @retval 2 Buffer handle is valid and buffer seems correct > > Another option is to set errno to indicate the reason of failure. Valid vs. > broken buffer/packet is the most valuable information from the call. It's > secondary to know why it's broken. No need to change return values or add > errno for v1.0. > > Also if this function needs to accept bad handles, I think it means that in > general the handle structure/mapping to memory/etc have to be error tolerant, > which we try to not force on other calls (by specifying undefined behavior on > bad handles). To me it sounds all or nothing - either all buffer calls (that > access only odp_buffer_t, no shared resources) have undefined behavior or non. > > So, I'd leave the patch out from the set, and improve documentation (add > "sanity check") with another patch. Agreed. The problem is still not well understood. But the current function definition is weak and liable to be misunderstood.
> >> >> > >> > So, I think this function could still crash on bad handles. Is there a >> use case that application would just want to check the handle validity >> (without crashing), but not the object sanity? >> I was thinking that some post-mortem dump code could iterate over all >> known (to the application) buffer and packet handles and save >> information about those. But in a crash situation, you can't really be >> sure that the handles you find are correct, e.g. could be stale >> handles so treated as invalid by an implementation. But you don't want >> to crash in your PMD code, in this case just save that this specific >> handle is invalid. > > Good point, but maybe post mortem tools are a separate topic (vendor > specific). Can we standardize what happens / can be done after crash? I guess > not. In my view, this post-mortem dumping is the responsibility of the application. ODP just needs to guarantee that I can (attempt to) inspect various ODP objects without crashing. Invalid handles, corrupt data structures etc should be reported back to the caller, not invoke undefined behavior. > > -Petri > >> >> > >> > -Petri >> > >> > _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
