diff mbox

[1/5] implement odp_bool type

Message ID 1417023121-19867-1-git-send-email-maxim.uvarov@linaro.org
State New
Headers show

Commit Message

Maxim Uvarov Nov. 26, 2014, 5:31 p.m. UTC
To have compatibility with other compilers define custom
booalen type for odp.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
---
 platform/linux-generic/include/api/odp_std_types.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Comments

Maxim Uvarov Nov. 27, 2014, 1:37 p.m. UTC | #1
On 11/26/2014 08:31 PM, Maxim Uvarov wrote:
> To have compatibility with other compilers define custom
> booalen type for odp.
>
> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> ---
>   platform/linux-generic/include/api/odp_std_types.h | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/platform/linux-generic/include/api/odp_std_types.h b/platform/linux-generic/include/api/odp_std_types.h
> index b12a2f3..e1f3971 100644
> --- a/platform/linux-generic/include/api/odp_std_types.h
> +++ b/platform/linux-generic/include/api/odp_std_types.h
> @@ -27,9 +27,10 @@ extern "C" {
>   #include <inttypes.h>
>   #include <limits.h>
>   
> -
> -
> -
> +/** odp_bool type used for compatibility of boolean type in
> + *  different compilers.
> + */
> +typedef uint32_t odp_bool;
odp_bool_t is better.

Can this be enum? I don't remember why we said that int is better then enum?

enum odp_bool_t {
     ODP_FALSE = 0,
     ODP_TRUE = 1,
}


Maxim.


>   
>   #ifdef __cplusplus
>   }
Ola Liljedahl Nov. 27, 2014, 4:47 p.m. UTC | #2
On 27 November 2014 at 14:37, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> On 11/26/2014 08:31 PM, Maxim Uvarov wrote:
>>
>> To have compatibility with other compilers define custom
>> booalen type for odp.
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
>> ---
>>   platform/linux-generic/include/api/odp_std_types.h | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/api/odp_std_types.h
>> b/platform/linux-generic/include/api/odp_std_types.h
>> index b12a2f3..e1f3971 100644
>> --- a/platform/linux-generic/include/api/odp_std_types.h
>> +++ b/platform/linux-generic/include/api/odp_std_types.h
>> @@ -27,9 +27,10 @@ extern "C" {
>>   #include <inttypes.h>
>>   #include <limits.h>
>>   -
>> -
>> -
>> +/** odp_bool type used for compatibility of boolean type in
>> + *  different compilers.
It's not necessary for compatibility with different compilers, we
could use stdbool.h for that (defined since C99 AFAIK).
We want the boolean type to have a well-defined and known size,
regardless which compiler is used as this facilities interoperability
between e.g. different compilers. Perhaps this is what you intended to
say.

>> + */
>> +typedef uint32_t odp_bool;
>
> odp_bool_t is better.
Yes.
But I would suggest the "standard" definition:
typedef int odp_bool_t;

>
> Can this be enum? I don't remember why we said that int is better then enum?
Now we are back in bool territory.
How large is an enum? Some compilers allow that to change, if the enum
range is small enough, a char or a short might be used instead of an
int.

>
> enum odp_bool_t {
>     ODP_FALSE = 0,
>     ODP_TRUE = 1,
> }
No need to define symbolic values for true and false. 0 means false,
non-zero means true, this is the semantics for conditionals in C. The
user can use whatever symbolic names they desire. stdbool.h false and
true can be used.

>
>
> Maxim.
>
>
>>     #ifdef __cplusplus
>>   }
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Maxim Uvarov Nov. 27, 2014, 5:38 p.m. UTC | #3
thanks Ola for explanation, will account that in new version.

Maxim.

On 27 November 2014 at 19:47, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> On 27 November 2014 at 14:37, Maxim Uvarov <maxim.uvarov@linaro.org>
> wrote:
> > On 11/26/2014 08:31 PM, Maxim Uvarov wrote:
> >>
> >> To have compatibility with other compilers define custom
> >> booalen type for odp.
> >>
> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>
> >> ---
> >>   platform/linux-generic/include/api/odp_std_types.h | 7 ++++---
> >>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/platform/linux-generic/include/api/odp_std_types.h
> >> b/platform/linux-generic/include/api/odp_std_types.h
> >> index b12a2f3..e1f3971 100644
> >> --- a/platform/linux-generic/include/api/odp_std_types.h
> >> +++ b/platform/linux-generic/include/api/odp_std_types.h
> >> @@ -27,9 +27,10 @@ extern "C" {
> >>   #include <inttypes.h>
> >>   #include <limits.h>
> >>   -
> >> -
> >> -
> >> +/** odp_bool type used for compatibility of boolean type in
> >> + *  different compilers.
> It's not necessary for compatibility with different compilers, we
> could use stdbool.h for that (defined since C99 AFAIK).
> We want the boolean type to have a well-defined and known size,
> regardless which compiler is used as this facilities interoperability
> between e.g. different compilers. Perhaps this is what you intended to
> say.
>
> >> + */
> >> +typedef uint32_t odp_bool;
> >
> > odp_bool_t is better.
> Yes.
> But I would suggest the "standard" definition:
> typedef int odp_bool_t;
>
> >
> > Can this be enum? I don't remember why we said that int is better then
> enum?
> Now we are back in bool territory.
> How large is an enum? Some compilers allow that to change, if the enum
> range is small enough, a char or a short might be used instead of an
> int.
>
> >
> > enum odp_bool_t {
> >     ODP_FALSE = 0,
> >     ODP_TRUE = 1,
> > }
> No need to define symbolic values for true and false. 0 means false,
> non-zero means true, this is the semantics for conditionals in C. The
> user can use whatever symbolic names they desire. stdbool.h false and
> true can be used.
>
> >
> >
> > Maxim.
> >
> >
> >>     #ifdef __cplusplus
> >>   }
> >
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
Bill Fischofer Nov. 27, 2014, 5:38 p.m. UTC | #4
I agree.  This type is supposed to be just documentation so

typedef int odp_bool_t;

is simplest and best.

Bill

On Thursday, November 27, 2014, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> On 27 November 2014 at 14:37, Maxim Uvarov <maxim.uvarov@linaro.org
> <javascript:;>> wrote:
> > On 11/26/2014 08:31 PM, Maxim Uvarov wrote:
> >>
> >> To have compatibility with other compilers define custom
> >> booalen type for odp.
> >>
> >> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org <javascript:;>>
> >> ---
> >>   platform/linux-generic/include/api/odp_std_types.h | 7 ++++---
> >>   1 file changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/platform/linux-generic/include/api/odp_std_types.h
> >> b/platform/linux-generic/include/api/odp_std_types.h
> >> index b12a2f3..e1f3971 100644
> >> --- a/platform/linux-generic/include/api/odp_std_types.h
> >> +++ b/platform/linux-generic/include/api/odp_std_types.h
> >> @@ -27,9 +27,10 @@ extern "C" {
> >>   #include <inttypes.h>
> >>   #include <limits.h>
> >>   -
> >> -
> >> -
> >> +/** odp_bool type used for compatibility of boolean type in
> >> + *  different compilers.
> It's not necessary for compatibility with different compilers, we
> could use stdbool.h for that (defined since C99 AFAIK).
> We want the boolean type to have a well-defined and known size,
> regardless which compiler is used as this facilities interoperability
> between e.g. different compilers. Perhaps this is what you intended to
> say.
>
> >> + */
> >> +typedef uint32_t odp_bool;
> >
> > odp_bool_t is better.
> Yes.
> But I would suggest the "standard" definition:
> typedef int odp_bool_t;
>
> >
> > Can this be enum? I don't remember why we said that int is better then
> enum?
> Now we are back in bool territory.
> How large is an enum? Some compilers allow that to change, if the enum
> range is small enough, a char or a short might be used instead of an
> int.
>
> >
> > enum odp_bool_t {
> >     ODP_FALSE = 0,
> >     ODP_TRUE = 1,
> > }
> No need to define symbolic values for true and false. 0 means false,
> non-zero means true, this is the semantics for conditionals in C. The
> user can use whatever symbolic names they desire. stdbool.h false and
> true can be used.
>
> >
> >
> > Maxim.
> >
> >
> >>     #ifdef __cplusplus
> >>   }
> >
> >
> >
> > _______________________________________________
> > lng-odp mailing list
> > lng-odp@lists.linaro.org <javascript:;>
> > http://lists.linaro.org/mailman/listinfo/lng-odp
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org <javascript:;>
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Taras Kondratiuk Nov. 28, 2014, 12:27 p.m. UTC | #5
On 11/27/2014 07:38 PM, Bill Fischofer wrote:
> I agree.  This type is supposed to be just documentation so
>
> typedef int odp_bool_t;
>
> is simplest and best.

Won't enum be more clear? {ODP_FALSE, ODP_TRUE}
Bill Fischofer Nov. 28, 2014, 12:42 p.m. UTC | #6
Not really.  int has been used for booleans for some time and the 0 =
false, 1 = true convention is well established.  The purpose of enums is to
support remapping.  Under no circumstances would one expect to see some
platform define ODP_TRUE to be anything other than 1 or ODP_FALSE to be
anything other than 0.  So in this case an enum is just adding syntactic
clutter for no benefit.

On Fri, Nov 28, 2014 at 7:27 AM, Taras Kondratiuk <
taras.kondratiuk@linaro.org> wrote:

> On 11/27/2014 07:38 PM, Bill Fischofer wrote:
>
>> I agree.  This type is supposed to be just documentation so
>>
>> typedef int odp_bool_t;
>>
>> is simplest and best.
>>
>
> Won't enum be more clear? {ODP_FALSE, ODP_TRUE}
>
Ola Liljedahl Nov. 28, 2014, 12:48 p.m. UTC | #7
And you can do
#define фальш 0
#define щоправда 1
in your code Taras.

On 28 November 2014 at 13:42, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> Not really.  int has been used for booleans for some time and the 0 = false,
> 1 = true convention is well established.  The purpose of enums is to support
> remapping.  Under no circumstances would one expect to see some platform
> define ODP_TRUE to be anything other than 1 or ODP_FALSE to be anything
> other than 0.  So in this case an enum is just adding syntactic clutter for
> no benefit.
>
> On Fri, Nov 28, 2014 at 7:27 AM, Taras Kondratiuk
> <taras.kondratiuk@linaro.org> wrote:
>>
>> On 11/27/2014 07:38 PM, Bill Fischofer wrote:
>>>
>>> I agree.  This type is supposed to be just documentation so
>>>
>>> typedef int odp_bool_t;
>>>
>>> is simplest and best.
>>
>>
>> Won't enum be more clear? {ODP_FALSE, ODP_TRUE}
>
>
Taras Kondratiuk Nov. 28, 2014, 1:13 p.m. UTC | #8
On 11/28/2014 02:42 PM, Bill Fischofer wrote:
> Not really.  int has been used for booleans for some time and the 0 =
> false, 1 = true convention is well established.  The purpose of enums is
> to support remapping.  Under no circumstances would one expect to see
> some platform define ODP_TRUE to be anything other than 1 or ODP_FALSE
> to be anything other than 0.  So in this case an enum is just adding
> syntactic clutter for no benefit.

In this case purpose of enums is not remapping, but value validation.

In different parts of our specification we have true = 1 or true = !0.
So it is not clear if '2' can be passed as 'true' to a function that had 
odp_bool_t argument. If true is exactly 1, then it would be more
clear to see ODP_TRUE as input value.
Ola Liljedahl Nov. 28, 2014, 1:25 p.m. UTC | #9
In C conditional expressions (as used by if, for, while, ? ), any
non-zero value means true so yes you can pass "2" to indicate true.
Using "true" or "TRUE" enums or preprocessor symbols that evaluate to
"1" is just to enhance readability and understanding (at least that's
the hope but then we're having this conversation...).

"#define TRUE 1" (or the corresponding enum definition) does not mean
that *only* "1" represents true.

If the ODP specification mentions (specifies) that *only* 1 can be
used as true, then this is wrong in my opinion and should be changed.

-- Ola


On 28 November 2014 at 14:13, Taras Kondratiuk
<taras.kondratiuk@linaro.org> wrote:
> On 11/28/2014 02:42 PM, Bill Fischofer wrote:
>>
>> Not really.  int has been used for booleans for some time and the 0 =
>> false, 1 = true convention is well established.  The purpose of enums is
>> to support remapping.  Under no circumstances would one expect to see
>> some platform define ODP_TRUE to be anything other than 1 or ODP_FALSE
>> to be anything other than 0.  So in this case an enum is just adding
>> syntactic clutter for no benefit.
>
>
> In this case purpose of enums is not remapping, but value validation.
>
> In different parts of our specification we have true = 1 or true = !0.
> So it is not clear if '2' can be passed as 'true' to a function that had
> odp_bool_t argument. If true is exactly 1, then it would be more
> clear to see ODP_TRUE as input value.
Bill Fischofer Nov. 28, 2014, 1:41 p.m. UTC | #10
ODP supports C and as such all normal C idioms apply.  False is 0 and true
is != false.  I don't see a need to overspecify beyond that.  For ODP
routines that return an odp_bool_t they return 1 for true.

On Fri, Nov 28, 2014 at 8:25 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
wrote:

> In C conditional expressions (as used by if, for, while, ? ), any
> non-zero value means true so yes you can pass "2" to indicate true.
> Using "true" or "TRUE" enums or preprocessor symbols that evaluate to
> "1" is just to enhance readability and understanding (at least that's
> the hope but then we're having this conversation...).
>
> "#define TRUE 1" (or the corresponding enum definition) does not mean
> that *only* "1" represents true.
>
> If the ODP specification mentions (specifies) that *only* 1 can be
> used as true, then this is wrong in my opinion and should be changed.
>
> -- Ola
>
>
> On 28 November 2014 at 14:13, Taras Kondratiuk
> <taras.kondratiuk@linaro.org> wrote:
> > On 11/28/2014 02:42 PM, Bill Fischofer wrote:
> >>
> >> Not really.  int has been used for booleans for some time and the 0 =
> >> false, 1 = true convention is well established.  The purpose of enums is
> >> to support remapping.  Under no circumstances would one expect to see
> >> some platform define ODP_TRUE to be anything other than 1 or ODP_FALSE
> >> to be anything other than 0.  So in this case an enum is just adding
> >> syntactic clutter for no benefit.
> >
> >
> > In this case purpose of enums is not remapping, but value validation.
> >
> > In different parts of our specification we have true = 1 or true = !0.
> > So it is not clear if '2' can be passed as 'true' to a function that had
> > odp_bool_t argument. If true is exactly 1, then it would be more
> > clear to see ODP_TRUE as input value.
>
Ola Liljedahl Nov. 28, 2014, 1:46 p.m. UTC | #11
On 28 November 2014 at 14:41, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> ODP supports C and as such all normal C idioms apply.  False is 0 and true
> is != false.  I don't see a need to overspecify beyond that.  For ODP
> routines that return an odp_bool_t they return 1 for true.
Agree. I was not perfectly clear below.

>
> On Fri, Nov 28, 2014 at 8:25 AM, Ola Liljedahl <ola.liljedahl@linaro.org>
> wrote:
>>
>> In C conditional expressions (as used by if, for, while, ? ), any
>> non-zero value means true so yes you can pass "2" to indicate true.
>> Using "true" or "TRUE" enums or preprocessor symbols that evaluate to
>> "1" is just to enhance readability and understanding (at least that's
>> the hope but then we're having this conversation...).
>>
>> "#define TRUE 1" (or the corresponding enum definition) does not mean
>> that *only* "1" represents true.
>>
>> If the ODP specification mentions (specifies) that *only* 1 can be
>> used as true, then this is wrong in my opinion and should be changed.
I meant for input (e.g. function parameters) here. For output (e.g.
functions returning boolean values), we should do as Bill specified
above and return 1.

>>
>> -- Ola
>>
>>
>> On 28 November 2014 at 14:13, Taras Kondratiuk
>> <taras.kondratiuk@linaro.org> wrote:
>> > On 11/28/2014 02:42 PM, Bill Fischofer wrote:
>> >>
>> >> Not really.  int has been used for booleans for some time and the 0 =
>> >> false, 1 = true convention is well established.  The purpose of enums
>> >> is
>> >> to support remapping.  Under no circumstances would one expect to see
>> >> some platform define ODP_TRUE to be anything other than 1 or ODP_FALSE
>> >> to be anything other than 0.  So in this case an enum is just adding
>> >> syntactic clutter for no benefit.
>> >
>> >
>> > In this case purpose of enums is not remapping, but value validation.
>> >
>> > In different parts of our specification we have true = 1 or true = !0.
>> > So it is not clear if '2' can be passed as 'true' to a function that had
>> > odp_bool_t argument. If true is exactly 1, then it would be more
>> > clear to see ODP_TRUE as input value.
>
>
Taras Kondratiuk Nov. 28, 2014, 1:59 p.m. UTC | #12
On 11/28/2014 03:46 PM, Ola Liljedahl wrote:
> On 28 November 2014 at 14:41, Bill Fischofer <bill.fischofer@linaro.org> wrote:
>> ODP supports C and as such all normal C idioms apply.  False is 0 and true
>> is != false.  I don't see a need to overspecify beyond that.  For ODP
>> routines that return an odp_bool_t they return 1 for true.
> Agree. I was not perfectly clear below.

That is my issue. Why should ODP over-specify 'true' and return exactly 
1 for true? Implementation will have to always convert !0 to 1 on
return.

Instead of:
odp_boot_t odp_packet_has_l3(odp_packet_t pkt)
{
	return flags(pkt) & l3_mask;
}

Implementation should do:
odp_boot_t odp_packet_has_l3(odp_packet_t pkt)
{
	return (flags(pkt) & l3_mask) ? 1 : 0;
}
diff mbox

Patch

diff --git a/platform/linux-generic/include/api/odp_std_types.h b/platform/linux-generic/include/api/odp_std_types.h
index b12a2f3..e1f3971 100644
--- a/platform/linux-generic/include/api/odp_std_types.h
+++ b/platform/linux-generic/include/api/odp_std_types.h
@@ -27,9 +27,10 @@  extern "C" {
 #include <inttypes.h>
 #include <limits.h>
 
-
-
-
+/** odp_bool type used for compatibility of boolean type in
+ *  different compilers.
+ */
+typedef uint32_t odp_bool;
 
 #ifdef __cplusplus
 }