> -----Original Message----- > From: ext Ola Liljedahl [mailto:[email protected]] > Sent: Wednesday, February 04, 2015 12:01 PM > To: Savolainen, Petri (NSN - FI/Espoo) > Cc: [email protected] > 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) > <[email protected]> wrote: > > > > > >> -----Original Message----- > >> From: [email protected] [mailto:lng-odp- > >> [email protected]] On Behalf Of ext Ola Liljedahl > >> Sent: Tuesday, February 03, 2015 6:48 PM > >> To: [email protected] > >> 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 <[email protected]> > >> --- > >> (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. > > > > > 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. -Petri > > > > > -Petri > > > > _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
