On 30 January 2015 at 15:40, Savolainen, Petri (NSN - FI/Espoo) <petri.savolai...@nsn.com> wrote: > > >> -----Original Message----- >> From: ext Taras Kondratiuk [mailto:taras.kondrat...@linaro.org] >> Sent: Friday, January 30, 2015 3:17 PM >> To: Savolainen, Petri (NSN - FI/Espoo) >> Cc: ext Ola Liljedahl; lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] Handling xxx_INVALID handles >> >> On 01/30/2015 02:41 PM, Savolainen, Petri (NSN - FI/Espoo) wrote: >> > >> > >> >> -----Original Message----- >> >> From: ext Taras Kondratiuk [mailto:taras.kondrat...@linaro.org] >> >> Sent: Friday, January 30, 2015 2:28 PM >> >> To: Savolainen, Petri (NSN - FI/Espoo); ext Ola Liljedahl >> >> Cc: lng-odp@lists.linaro.org >> >> Subject: Re: [lng-odp] Handling xxx_INVALID handles >> >> >> >> On 01/30/2015 02:06 PM, Savolainen, Petri (NSN - FI/Espoo) wrote: >> >>> >> >>> >> >>>> -----Original Message----- >> >>>> From: ext Taras Kondratiuk [mailto:taras.kondrat...@linaro.org] >> >>>> Sent: Friday, January 30, 2015 1:50 PM >> >>>> To: Savolainen, Petri (NSN - FI/Espoo); ext Ola Liljedahl >> >>>> Cc: lng-odp@lists.linaro.org >> >>>> Subject: Re: [lng-odp] Handling xxx_INVALID handles >> >>>> >> >>>> On 01/30/2015 01:15 PM, Savolainen, Petri (NSN - FI/Espoo) wrote: >> >>>>> As explained below, XXX_NULL != XXX_INVALID enables to distinguish >> >> this >> >>>> error ... >> >>>>> >> >>>>> x = odp_xxx_create(...); // error! => Returns XXX_INVALID >> >>>>> y = odp_yyy_create(x, ....); // x == XXX_INVALID => error! >> >>>> >> >>>> Deferring error detection to a second function doesn't look like a >> good >> >>>> practice. It will be harder to determine a cause of the error then. >> >>> >> >>> The "error!" mains any, undefined error (e.g. crash). It's an example >> of >> >> an error, another would be this. >> >>> >> >>> x = XXX_INVALID; //init >> >>> >> >>> // forgot to set x with valid value >> >>> >> >>> y = odp_yyy_create(x, ....); // x == XXX_INVALID => error! >> >>> >> >>> >> >>>> >> >>>> Also 'x == XXX_INVALID => error!' is not a correct statement. >> >>>> We agreed that 'x == XXX_INVALID => *undefined behavior*', so this is >> a >> >>>> definitely a bad example. >> >>> >> >>> Error! == undefined error, crash! >> >> >> >> 'Undefined behavior' means not only error return or crash, but it can >> >> silently succeed and mess some unrelated stuff. So application will >> >> crash later in some random place. Do you want to promote such API >> usage? >> >> >> >> Sorry, but I'm really missing your point. >> > >> > >> > The point is that, if we use XXX_INVALID to indicate that user choose to >> let the implementation decide on parameter X (e.g. let pool_create() to >> reserve the shared memory), how implementation would separate that >> intention from a bad handle (like XXX_INVALID)? >> > >> > We reserve special handle id XXX_NULL for that purpose only. When user >> gives that implementation, do not crash etc, but knows that it has to >> reserve the resource X. >> > >> > Behaviour is undefined: >> > odp_xxx_create(XXX_INVALID, ..) >> > >> > Behaviour is defined: >> > odp_xxx_create(XXX_NULL, ..) >> >> I understand that part, but if I try to list possible use cases which >> don't cause undefined behavior, then XXX_INVALID seems to be redundant >> anyway. >> >> Let's take your example >> >> >>>>> x = odp_xxx_create(...); // error! => Returns XXX_INVALID >> >>>>> y = odp_yyy_create(x, ....); // x == XXX_INVALID => error! >> >> - If x can be XXX_INVALID, then application have to *always* check it >> before passing to odp_yyy_create(). Otherwise - undefined behavior. >> - If application have to check it anyway, then it can test it against >> ODP_XXX_NULL -> odp_xxx_create() can return _NULL instead of _INVALID. >> - If x is ODP_XXX_NULL, then odp_yyy_create() should not be called. >> - No need for ODP_XXX_INVALID. >> >> Can you give an example which doesn't cause undefined behavior where >> both XXX_IVALID and XXX_NULL are needed? > > The example above is an example of bad application. Of course application > should check x before passing it forward, but when it _fails_ to do so (or > inits it but fails to set it with a valid handle), we are still possible to > catch that. Contrived examples. Or do you claim these are actual code snippets from NSN applications?
I don't think these use cases can be supported unless we define that ODP calls must detect INVALID handles and handle them in a well-defined manner. The question is which calls, perhaps all of them. > > If we use the same value as "invalid" return value and "null option", we > cannot even check for the error above. > > > 1) > > x = odp_xxx_create(...); > // Should check x here, but missed it. > // Robust implementation should catch bad x, instead of > // letting odp_yyy_create() to succeed always with implementation > // reserved resource x. > // Implementation is in control of picking value for XXX_INVALID. > y = odp_yyy_create(x, ..); > > > 2) > > x = XXX_INVALID; // Intention is to stop if we don't create x > > if (0) > x = odp_xxx_create(...); > > if (x == XXX_INVALID) > ABORT; > else > ... // oops, forgot to call odp_xxx_create(...) > > // Failed to call odp_xxx_create(). Again implementation > // is possible to catch this, if XXX_NULL != XXX_INVALID. > y = odp_yyy_create(x, ..); > > > 3) > x = XXX_NULL; // Intention is to use default if we don't create x > > if (0) > x = odp_xxx_create(...); > > if (x == XXX_INVALID) > ABORT; > else > ... // Use default > > y = odp_yyy_create(x, ..); > > > > 1) and 2) have bugs and user wants to find those as early as possible. > Why API should force odp_yyy_create() to always succeed in 1) and 2)? > > It was not user's intention to continue there (with default). He wants to > catch the bug early - either through crash in odp_yyy_create() or YYY_INVALID > return value. > > Successful return from odp_yyy_create(XXX_INVALID, ...) would demonstrate bad > design, although is included in "undefined behavior". XXX_INVALID is special > value that is easy to check. > > > -Petri > > _______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
