Message ID | 1404838113-59583-1-git-send-email-mike.holmes@linaro.org |
---|---|
State | Rejected |
Headers | show |
Does not this make C99 absolute requirement for ODP library. I wonder whether it is too strict. C89/90 do not have bool type. Thanks, Victor On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > --- > include/odp_buffer.h | 4 ++-- > include/odp_coremask.h | 11 ++++++++--- > include/odp_ticketlock.h | 4 ++-- > platform/linux-generic/odp_buffer.c | 2 +- > platform/linux-generic/odp_coremask.c | 2 +- > platform/linux-generic/odp_ticketlock.c | 2 +- > 6 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/include/odp_buffer.h b/include/odp_buffer.h > index d8577fd..c1c9bef 100644 > --- a/include/odp_buffer.h > +++ b/include/odp_buffer.h > @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); > * > * @param buf Buffer handle > * > - * @return 1 if valid, otherwise 0 > + * @return true if valid, otherwise false > */ > -int odp_buffer_is_valid(odp_buffer_t buf); > +bool odp_buffer_is_valid(odp_buffer_t buf); > > /** > * Print buffer metadata to STDOUT > diff --git a/include/odp_coremask.h b/include/odp_coremask.h > index 141cb6a..f5a9a0b 100644 > --- a/include/odp_coremask.h > +++ b/include/odp_coremask.h > @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t *mask); > * Test if core is a member of mask > * @param core Core number > * @param mask Core mask to check if core num set or not > - * @return non-zero if set otherwise 0 > + * @return true if the core mask is set, otherwise false > */ > -int odp_coremask_isset(int core, const odp_coremask_t *mask); > +bool odp_coremask_isset(int core, const odp_coremask_t *mask); > > /** > * Count number of cores in mask > @@ -163,8 +163,13 @@ static inline void odp_coremask_xor(odp_coremask_t *dest, odp_coremask_t *src1, > > /** > * Test if two masks contain the same cores > + * > + * @param mask1 First mask to compare > + * @param mask2 Second mask to compare > + * > + * @return true is they are equal or false otherwise > */ > -static inline int odp_coremask_equal(odp_coremask_t *mask1, > +static inline bool odp_coremask_equal(odp_coremask_t *mask1, > odp_coremask_t *mask2) > { > return (mask1->_u64[0] == mask2->_u64[0]); > diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h > index 6277a18..2e61ef6 100644 > --- a/include/odp_ticketlock.h > +++ b/include/odp_ticketlock.h > @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock); > * > * @param ticketlock Ticketlock > * > - * @return 1 if the lock is locked, otherwise 0. > + * @return true if the ticket lock is locked, otherwise false > */ > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); > > > #ifdef __cplusplus > diff --git a/platform/linux-generic/odp_buffer.c b/platform/linux-generic/odp_buffer.c > index 0169eec..41b6a4c 100644 > --- a/platform/linux-generic/odp_buffer.c > +++ b/platform/linux-generic/odp_buffer.c > @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) > } > > > -int odp_buffer_is_valid(odp_buffer_t buf) > +bool odp_buffer_is_valid(odp_buffer_t buf) > { > odp_buffer_bits_t handle; > > diff --git a/platform/linux-generic/odp_coremask.c b/platform/linux-generic/odp_coremask.c > index c55eb72..c98a10a 100644 > --- a/platform/linux-generic/odp_coremask.c > +++ b/platform/linux-generic/odp_coremask.c > @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t *mask) > } > > > -int odp_coremask_isset(int core, const odp_coremask_t *mask) > +bool odp_coremask_isset(int core, const odp_coremask_t *mask) > { > /* should not be more than 63 > * core no. should be from 0..63= 64bit > diff --git a/platform/linux-generic/odp_ticketlock.c b/platform/linux-generic/odp_ticketlock.c > index be5b885..6cc5285 100644 > --- a/platform/linux-generic/odp_ticketlock.c > +++ b/platform/linux-generic/odp_ticketlock.c > @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock) > } > > > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) > { > return ticketlock->cur_ticket != ticketlock->next_ticket; > } > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
True, but I believe we chose this as our standard, although again our published arch does does not state that yet. A quick grep of our mailing list shows accepting patches to come into compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards -std=c99 compliance" Mike On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> wrote: > Does not this make C99 absolute requirement for > ODP library. I wonder whether it is too strict. > > C89/90 do not have bool type. > > Thanks, > Victor > > > On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: > > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > > --- > > include/odp_buffer.h | 4 ++-- > > include/odp_coremask.h | 11 ++++++++--- > > include/odp_ticketlock.h | 4 ++-- > > platform/linux-generic/odp_buffer.c | 2 +- > > platform/linux-generic/odp_coremask.c | 2 +- > > platform/linux-generic/odp_ticketlock.c | 2 +- > > 6 files changed, 15 insertions(+), 10 deletions(-) > > > > diff --git a/include/odp_buffer.h b/include/odp_buffer.h > > index d8577fd..c1c9bef 100644 > > --- a/include/odp_buffer.h > > +++ b/include/odp_buffer.h > > @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); > > * > > * @param buf Buffer handle > > * > > - * @return 1 if valid, otherwise 0 > > + * @return true if valid, otherwise false > > */ > > -int odp_buffer_is_valid(odp_buffer_t buf); > > +bool odp_buffer_is_valid(odp_buffer_t buf); > > > > /** > > * Print buffer metadata to STDOUT > > diff --git a/include/odp_coremask.h b/include/odp_coremask.h > > index 141cb6a..f5a9a0b 100644 > > --- a/include/odp_coremask.h > > +++ b/include/odp_coremask.h > > @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t > *mask); > > * Test if core is a member of mask > > * @param core Core number > > * @param mask Core mask to check if core num set or not > > - * @return non-zero if set otherwise 0 > > + * @return true if the core mask is set, otherwise false > > */ > > -int odp_coremask_isset(int core, const odp_coremask_t *mask); > > +bool odp_coremask_isset(int core, const odp_coremask_t *mask); > > > > /** > > * Count number of cores in mask > > @@ -163,8 +163,13 @@ static inline void odp_coremask_xor(odp_coremask_t > *dest, odp_coremask_t *src1, > > > > /** > > * Test if two masks contain the same cores > > + * > > + * @param mask1 First mask to compare > > + * @param mask2 Second mask to compare > > + * > > + * @return true is they are equal or false otherwise > > */ > > -static inline int odp_coremask_equal(odp_coremask_t *mask1, > > +static inline bool odp_coremask_equal(odp_coremask_t *mask1, > > odp_coremask_t *mask2) > > { > > return (mask1->_u64[0] == mask2->_u64[0]); > > diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h > > index 6277a18..2e61ef6 100644 > > --- a/include/odp_ticketlock.h > > +++ b/include/odp_ticketlock.h > > @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t > *ticketlock); > > * > > * @param ticketlock Ticketlock > > * > > - * @return 1 if the lock is locked, otherwise 0. > > + * @return true if the ticket lock is locked, otherwise false > > */ > > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); > > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); > > > > > > #ifdef __cplusplus > > diff --git a/platform/linux-generic/odp_buffer.c > b/platform/linux-generic/odp_buffer.c > > index 0169eec..41b6a4c 100644 > > --- a/platform/linux-generic/odp_buffer.c > > +++ b/platform/linux-generic/odp_buffer.c > > @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) > > } > > > > > > -int odp_buffer_is_valid(odp_buffer_t buf) > > +bool odp_buffer_is_valid(odp_buffer_t buf) > > { > > odp_buffer_bits_t handle; > > > > diff --git a/platform/linux-generic/odp_coremask.c > b/platform/linux-generic/odp_coremask.c > > index c55eb72..c98a10a 100644 > > --- a/platform/linux-generic/odp_coremask.c > > +++ b/platform/linux-generic/odp_coremask.c > > @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t *mask) > > } > > > > > > -int odp_coremask_isset(int core, const odp_coremask_t *mask) > > +bool odp_coremask_isset(int core, const odp_coremask_t *mask) > > { > > /* should not be more than 63 > > * core no. should be from 0..63= 64bit > > diff --git a/platform/linux-generic/odp_ticketlock.c > b/platform/linux-generic/odp_ticketlock.c > > index be5b885..6cc5285 100644 > > --- a/platform/linux-generic/odp_ticketlock.c > > +++ b/platform/linux-generic/odp_ticketlock.c > > @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t > *ticketlock) > > } > > > > > > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) > > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) > > { > > return ticketlock->cur_ticket != ticketlock->next_ticket; > > } > > -- > > 1.9.1 > > > > > > _______________________________________________ > > lng-odp mailing list > > lng-odp@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/lng-odp >
ODP has stated that we require C99. We'd like to move towards C11 but for many that's a bit too aggressive. C99 is now 15 years old so that should be mature enough for anyone. Bill On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > True, but I believe we chose this as our standard, although again our > published arch does does not state that yet. > > A quick grep of our mailing list shows accepting patches to come into > compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards -std=c99 > compliance" > > Mike > > > On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> wrote: > >> Does not this make C99 absolute requirement for >> ODP library. I wonder whether it is too strict. >> >> C89/90 do not have bool type. >> >> Thanks, >> Victor >> >> >> On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: >> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >> > --- >> > include/odp_buffer.h | 4 ++-- >> > include/odp_coremask.h | 11 ++++++++--- >> > include/odp_ticketlock.h | 4 ++-- >> > platform/linux-generic/odp_buffer.c | 2 +- >> > platform/linux-generic/odp_coremask.c | 2 +- >> > platform/linux-generic/odp_ticketlock.c | 2 +- >> > 6 files changed, 15 insertions(+), 10 deletions(-) >> > >> > diff --git a/include/odp_buffer.h b/include/odp_buffer.h >> > index d8577fd..c1c9bef 100644 >> > --- a/include/odp_buffer.h >> > +++ b/include/odp_buffer.h >> > @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); >> > * >> > * @param buf Buffer handle >> > * >> > - * @return 1 if valid, otherwise 0 >> > + * @return true if valid, otherwise false >> > */ >> > -int odp_buffer_is_valid(odp_buffer_t buf); >> > +bool odp_buffer_is_valid(odp_buffer_t buf); >> > >> > /** >> > * Print buffer metadata to STDOUT >> > diff --git a/include/odp_coremask.h b/include/odp_coremask.h >> > index 141cb6a..f5a9a0b 100644 >> > --- a/include/odp_coremask.h >> > +++ b/include/odp_coremask.h >> > @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t >> *mask); >> > * Test if core is a member of mask >> > * @param core Core number >> > * @param mask Core mask to check if core num set or not >> > - * @return non-zero if set otherwise 0 >> > + * @return true if the core mask is set, otherwise false >> > */ >> > -int odp_coremask_isset(int core, const odp_coremask_t *mask); >> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask); >> > >> > /** >> > * Count number of cores in mask >> > @@ -163,8 +163,13 @@ static inline void odp_coremask_xor(odp_coremask_t >> *dest, odp_coremask_t *src1, >> > >> > /** >> > * Test if two masks contain the same cores >> > + * >> > + * @param mask1 First mask to compare >> > + * @param mask2 Second mask to compare >> > + * >> > + * @return true is they are equal or false otherwise >> > */ >> > -static inline int odp_coremask_equal(odp_coremask_t *mask1, >> > +static inline bool odp_coremask_equal(odp_coremask_t *mask1, >> > odp_coremask_t *mask2) >> > { >> > return (mask1->_u64[0] == mask2->_u64[0]); >> > diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h >> > index 6277a18..2e61ef6 100644 >> > --- a/include/odp_ticketlock.h >> > +++ b/include/odp_ticketlock.h >> > @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t >> *ticketlock); >> > * >> > * @param ticketlock Ticketlock >> > * >> > - * @return 1 if the lock is locked, otherwise 0. >> > + * @return true if the ticket lock is locked, otherwise false >> > */ >> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >> > >> > >> > #ifdef __cplusplus >> > diff --git a/platform/linux-generic/odp_buffer.c >> b/platform/linux-generic/odp_buffer.c >> > index 0169eec..41b6a4c 100644 >> > --- a/platform/linux-generic/odp_buffer.c >> > +++ b/platform/linux-generic/odp_buffer.c >> > @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) >> > } >> > >> > >> > -int odp_buffer_is_valid(odp_buffer_t buf) >> > +bool odp_buffer_is_valid(odp_buffer_t buf) >> > { >> > odp_buffer_bits_t handle; >> > >> > diff --git a/platform/linux-generic/odp_coremask.c >> b/platform/linux-generic/odp_coremask.c >> > index c55eb72..c98a10a 100644 >> > --- a/platform/linux-generic/odp_coremask.c >> > +++ b/platform/linux-generic/odp_coremask.c >> > @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t *mask) >> > } >> > >> > >> > -int odp_coremask_isset(int core, const odp_coremask_t *mask) >> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask) >> > { >> > /* should not be more than 63 >> > * core no. should be from 0..63= 64bit >> > diff --git a/platform/linux-generic/odp_ticketlock.c >> b/platform/linux-generic/odp_ticketlock.c >> > index be5b885..6cc5285 100644 >> > --- a/platform/linux-generic/odp_ticketlock.c >> > +++ b/platform/linux-generic/odp_ticketlock.c >> > @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t >> *ticketlock) >> > } >> > >> > >> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >> > { >> > return ticketlock->cur_ticket != ticketlock->next_ticket; >> > } >> > -- >> > 1.9.1 >> > >> > >> > _______________________________________________ >> > lng-odp mailing list >> > lng-odp@lists.linaro.org >> > http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
FYI make CFLAGS=-std=c99 fails in the linux-generic odp_timer.c implementation. I know it is the API we say is C99 based and implementations have a lot of flexibility, but I feel linux-generic should also be C99 clean, hopefully Olas new version of generic timers will clean this up and we can make the C99 flag part of the build. Mike On 8 July 2014 13:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: > ODP has stated that we require C99. We'd like to move towards C11 but for > many that's a bit too aggressive. C99 is now 15 years old so that should > be mature enough for anyone. > > Bill > > > On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> True, but I believe we chose this as our standard, although again our >> published arch does does not state that yet. >> >> A quick grep of our mailing list shows accepting patches to come into >> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards -std=c99 >> compliance" >> >> Mike >> >> >> On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> wrote: >> >>> Does not this make C99 absolute requirement for >>> ODP library. I wonder whether it is too strict. >>> >>> C89/90 do not have bool type. >>> >>> Thanks, >>> Victor >>> >>> >>> On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: >>> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>> > --- >>> > include/odp_buffer.h | 4 ++-- >>> > include/odp_coremask.h | 11 ++++++++--- >>> > include/odp_ticketlock.h | 4 ++-- >>> > platform/linux-generic/odp_buffer.c | 2 +- >>> > platform/linux-generic/odp_coremask.c | 2 +- >>> > platform/linux-generic/odp_ticketlock.c | 2 +- >>> > 6 files changed, 15 insertions(+), 10 deletions(-) >>> > >>> > diff --git a/include/odp_buffer.h b/include/odp_buffer.h >>> > index d8577fd..c1c9bef 100644 >>> > --- a/include/odp_buffer.h >>> > +++ b/include/odp_buffer.h >>> > @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); >>> > * >>> > * @param buf Buffer handle >>> > * >>> > - * @return 1 if valid, otherwise 0 >>> > + * @return true if valid, otherwise false >>> > */ >>> > -int odp_buffer_is_valid(odp_buffer_t buf); >>> > +bool odp_buffer_is_valid(odp_buffer_t buf); >>> > >>> > /** >>> > * Print buffer metadata to STDOUT >>> > diff --git a/include/odp_coremask.h b/include/odp_coremask.h >>> > index 141cb6a..f5a9a0b 100644 >>> > --- a/include/odp_coremask.h >>> > +++ b/include/odp_coremask.h >>> > @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t >>> *mask); >>> > * Test if core is a member of mask >>> > * @param core Core number >>> > * @param mask Core mask to check if core num set or not >>> > - * @return non-zero if set otherwise 0 >>> > + * @return true if the core mask is set, otherwise false >>> > */ >>> > -int odp_coremask_isset(int core, const odp_coremask_t *mask); >>> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask); >>> > >>> > /** >>> > * Count number of cores in mask >>> > @@ -163,8 +163,13 @@ static inline void >>> odp_coremask_xor(odp_coremask_t *dest, odp_coremask_t *src1, >>> > >>> > /** >>> > * Test if two masks contain the same cores >>> > + * >>> > + * @param mask1 First mask to compare >>> > + * @param mask2 Second mask to compare >>> > + * >>> > + * @return true is they are equal or false otherwise >>> > */ >>> > -static inline int odp_coremask_equal(odp_coremask_t *mask1, >>> > +static inline bool odp_coremask_equal(odp_coremask_t *mask1, >>> > odp_coremask_t *mask2) >>> > { >>> > return (mask1->_u64[0] == mask2->_u64[0]); >>> > diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h >>> > index 6277a18..2e61ef6 100644 >>> > --- a/include/odp_ticketlock.h >>> > +++ b/include/odp_ticketlock.h >>> > @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>> *ticketlock); >>> > * >>> > * @param ticketlock Ticketlock >>> > * >>> > - * @return 1 if the lock is locked, otherwise 0. >>> > + * @return true if the ticket lock is locked, otherwise false >>> > */ >>> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>> > >>> > >>> > #ifdef __cplusplus >>> > diff --git a/platform/linux-generic/odp_buffer.c >>> b/platform/linux-generic/odp_buffer.c >>> > index 0169eec..41b6a4c 100644 >>> > --- a/platform/linux-generic/odp_buffer.c >>> > +++ b/platform/linux-generic/odp_buffer.c >>> > @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) >>> > } >>> > >>> > >>> > -int odp_buffer_is_valid(odp_buffer_t buf) >>> > +bool odp_buffer_is_valid(odp_buffer_t buf) >>> > { >>> > odp_buffer_bits_t handle; >>> > >>> > diff --git a/platform/linux-generic/odp_coremask.c >>> b/platform/linux-generic/odp_coremask.c >>> > index c55eb72..c98a10a 100644 >>> > --- a/platform/linux-generic/odp_coremask.c >>> > +++ b/platform/linux-generic/odp_coremask.c >>> > @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t *mask) >>> > } >>> > >>> > >>> > -int odp_coremask_isset(int core, const odp_coremask_t *mask) >>> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask) >>> > { >>> > /* should not be more than 63 >>> > * core no. should be from 0..63= 64bit >>> > diff --git a/platform/linux-generic/odp_ticketlock.c >>> b/platform/linux-generic/odp_ticketlock.c >>> > index be5b885..6cc5285 100644 >>> > --- a/platform/linux-generic/odp_ticketlock.c >>> > +++ b/platform/linux-generic/odp_ticketlock.c >>> > @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>> *ticketlock) >>> > } >>> > >>> > >>> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>> > { >>> > return ticketlock->cur_ticket != ticketlock->next_ticket; >>> > } >>> > -- >>> > 1.9.1 >>> > >>> > >>> > _______________________________________________ >>> > lng-odp mailing list >>> > lng-odp@lists.linaro.org >>> > http://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> >> >> -- >> *Mike Holmes* >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >
Agreed. This is one of those "cleanup" areas that testing needs to focus on. Anything we can do along the way helps avoid a bigger issue in Q4. Bill On Tue, Jul 8, 2014 at 12:37 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > FYI make CFLAGS=-std=c99 fails in the linux-generic odp_timer.c > implementation. > > I know it is the API we say is C99 based and implementations have a lot of > flexibility, but I feel linux-generic should also be C99 clean, hopefully > Olas new version of generic timers will clean this up and we can make the > C99 flag part of the build. > > Mike > > > On 8 July 2014 13:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: > >> ODP has stated that we require C99. We'd like to move towards C11 but >> for many that's a bit too aggressive. C99 is now 15 years old so that >> should be mature enough for anyone. >> >> Bill >> >> >> On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes <mike.holmes@linaro.org> >> wrote: >> >>> True, but I believe we chose this as our standard, although again our >>> published arch does does not state that yet. >>> >>> A quick grep of our mailing list shows accepting patches to come into >>> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards -std=c99 >>> compliance" >>> >>> Mike >>> >>> >>> On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> >>> wrote: >>> >>>> Does not this make C99 absolute requirement for >>>> ODP library. I wonder whether it is too strict. >>>> >>>> C89/90 do not have bool type. >>>> >>>> Thanks, >>>> Victor >>>> >>>> >>>> On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: >>>> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>>> > --- >>>> > include/odp_buffer.h | 4 ++-- >>>> > include/odp_coremask.h | 11 ++++++++--- >>>> > include/odp_ticketlock.h | 4 ++-- >>>> > platform/linux-generic/odp_buffer.c | 2 +- >>>> > platform/linux-generic/odp_coremask.c | 2 +- >>>> > platform/linux-generic/odp_ticketlock.c | 2 +- >>>> > 6 files changed, 15 insertions(+), 10 deletions(-) >>>> > >>>> > diff --git a/include/odp_buffer.h b/include/odp_buffer.h >>>> > index d8577fd..c1c9bef 100644 >>>> > --- a/include/odp_buffer.h >>>> > +++ b/include/odp_buffer.h >>>> > @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); >>>> > * >>>> > * @param buf Buffer handle >>>> > * >>>> > - * @return 1 if valid, otherwise 0 >>>> > + * @return true if valid, otherwise false >>>> > */ >>>> > -int odp_buffer_is_valid(odp_buffer_t buf); >>>> > +bool odp_buffer_is_valid(odp_buffer_t buf); >>>> > >>>> > /** >>>> > * Print buffer metadata to STDOUT >>>> > diff --git a/include/odp_coremask.h b/include/odp_coremask.h >>>> > index 141cb6a..f5a9a0b 100644 >>>> > --- a/include/odp_coremask.h >>>> > +++ b/include/odp_coremask.h >>>> > @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t >>>> *mask); >>>> > * Test if core is a member of mask >>>> > * @param core Core number >>>> > * @param mask Core mask to check if core num set or not >>>> > - * @return non-zero if set otherwise 0 >>>> > + * @return true if the core mask is set, otherwise false >>>> > */ >>>> > -int odp_coremask_isset(int core, const odp_coremask_t *mask); >>>> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask); >>>> > >>>> > /** >>>> > * Count number of cores in mask >>>> > @@ -163,8 +163,13 @@ static inline void >>>> odp_coremask_xor(odp_coremask_t *dest, odp_coremask_t *src1, >>>> > >>>> > /** >>>> > * Test if two masks contain the same cores >>>> > + * >>>> > + * @param mask1 First mask to compare >>>> > + * @param mask2 Second mask to compare >>>> > + * >>>> > + * @return true is they are equal or false otherwise >>>> > */ >>>> > -static inline int odp_coremask_equal(odp_coremask_t *mask1, >>>> > +static inline bool odp_coremask_equal(odp_coremask_t *mask1, >>>> > odp_coremask_t *mask2) >>>> > { >>>> > return (mask1->_u64[0] == mask2->_u64[0]); >>>> > diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h >>>> > index 6277a18..2e61ef6 100644 >>>> > --- a/include/odp_ticketlock.h >>>> > +++ b/include/odp_ticketlock.h >>>> > @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>>> *ticketlock); >>>> > * >>>> > * @param ticketlock Ticketlock >>>> > * >>>> > - * @return 1 if the lock is locked, otherwise 0. >>>> > + * @return true if the ticket lock is locked, otherwise false >>>> > */ >>>> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>>> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>>> > >>>> > >>>> > #ifdef __cplusplus >>>> > diff --git a/platform/linux-generic/odp_buffer.c >>>> b/platform/linux-generic/odp_buffer.c >>>> > index 0169eec..41b6a4c 100644 >>>> > --- a/platform/linux-generic/odp_buffer.c >>>> > +++ b/platform/linux-generic/odp_buffer.c >>>> > @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) >>>> > } >>>> > >>>> > >>>> > -int odp_buffer_is_valid(odp_buffer_t buf) >>>> > +bool odp_buffer_is_valid(odp_buffer_t buf) >>>> > { >>>> > odp_buffer_bits_t handle; >>>> > >>>> > diff --git a/platform/linux-generic/odp_coremask.c >>>> b/platform/linux-generic/odp_coremask.c >>>> > index c55eb72..c98a10a 100644 >>>> > --- a/platform/linux-generic/odp_coremask.c >>>> > +++ b/platform/linux-generic/odp_coremask.c >>>> > @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t >>>> *mask) >>>> > } >>>> > >>>> > >>>> > -int odp_coremask_isset(int core, const odp_coremask_t *mask) >>>> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask) >>>> > { >>>> > /* should not be more than 63 >>>> > * core no. should be from 0..63= 64bit >>>> > diff --git a/platform/linux-generic/odp_ticketlock.c >>>> b/platform/linux-generic/odp_ticketlock.c >>>> > index be5b885..6cc5285 100644 >>>> > --- a/platform/linux-generic/odp_ticketlock.c >>>> > +++ b/platform/linux-generic/odp_ticketlock.c >>>> > @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>>> *ticketlock) >>>> > } >>>> > >>>> > >>>> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>>> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>>> > { >>>> > return ticketlock->cur_ticket != ticketlock->next_ticket; >>>> > } >>>> > -- >>>> > 1.9.1 >>>> > >>>> > >>>> > _______________________________________________ >>>> > lng-odp mailing list >>>> > lng-odp@lists.linaro.org >>>> > http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>> >>> >>> >>> -- >>> *Mike Holmes* >>> Linaro Technical Manager / Lead >>> LNG - ODP >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >> > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP >
On 8 July 2014 10:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: > ODP has stated that we require C99. Sorry, I don't recall that. I remember that C99 compliance was discussed and I don't have anything against it. C99 compliance does not prevent code to be C89/C90 compliant. I.e one may state if C99 compliant client uses ODP library it should work seamlessly. Have requirement for client of ODP library to be C99 compliant is quite different matter. Note what compliance level implementation of ODP library uses inside does not matter. I care about requirements to client code that would include ODP library header files. > We'd like to move towards C11 but for > many that's a bit too aggressive. C99 is now 15 years old so that should be > mature enough for anyone. I disagree, there is code much older than that :). In practice old code may have its own #definition of bool. So it will be quite messy. Although it may sort of work accidentally. Other way is to #include <stdbool.h> gcc provides bool define there for older not C99 compliant code, but I suspect it could be gcc specific and still will mess up with "other" bool definitions. I prefer that client visible header files would not use bool type. Thanks, Victor > Bill > > > On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes <mike.holmes@linaro.org> wrote: >> >> True, but I believe we chose this as our standard, although again our >> published arch does does not state that yet. >> >> A quick grep of our mailing list shows accepting patches to come into >> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards -std=c99 >> compliance" >> >> Mike >> >> >> On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> wrote: >>> >>> Does not this make C99 absolute requirement for >>> ODP library. I wonder whether it is too strict. >>> >>> C89/90 do not have bool type. >>> >>> Thanks, >>> Victor >>> >>> >>> On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: >>> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>> > --- >>> > include/odp_buffer.h | 4 ++-- >>> > include/odp_coremask.h | 11 ++++++++--- >>> > include/odp_ticketlock.h | 4 ++-- >>> > platform/linux-generic/odp_buffer.c | 2 +- >>> > platform/linux-generic/odp_coremask.c | 2 +- >>> > platform/linux-generic/odp_ticketlock.c | 2 +- >>> > 6 files changed, 15 insertions(+), 10 deletions(-) >>> > >>> > diff --git a/include/odp_buffer.h b/include/odp_buffer.h >>> > index d8577fd..c1c9bef 100644 >>> > --- a/include/odp_buffer.h >>> > +++ b/include/odp_buffer.h >>> > @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); >>> > * >>> > * @param buf Buffer handle >>> > * >>> > - * @return 1 if valid, otherwise 0 >>> > + * @return true if valid, otherwise false >>> > */ >>> > -int odp_buffer_is_valid(odp_buffer_t buf); >>> > +bool odp_buffer_is_valid(odp_buffer_t buf); >>> > >>> > /** >>> > * Print buffer metadata to STDOUT >>> > diff --git a/include/odp_coremask.h b/include/odp_coremask.h >>> > index 141cb6a..f5a9a0b 100644 >>> > --- a/include/odp_coremask.h >>> > +++ b/include/odp_coremask.h >>> > @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t >>> > *mask); >>> > * Test if core is a member of mask >>> > * @param core Core number >>> > * @param mask Core mask to check if core num set or not >>> > - * @return non-zero if set otherwise 0 >>> > + * @return true if the core mask is set, otherwise false >>> > */ >>> > -int odp_coremask_isset(int core, const odp_coremask_t *mask); >>> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask); >>> > >>> > /** >>> > * Count number of cores in mask >>> > @@ -163,8 +163,13 @@ static inline void odp_coremask_xor(odp_coremask_t >>> > *dest, odp_coremask_t *src1, >>> > >>> > /** >>> > * Test if two masks contain the same cores >>> > + * >>> > + * @param mask1 First mask to compare >>> > + * @param mask2 Second mask to compare >>> > + * >>> > + * @return true is they are equal or false otherwise >>> > */ >>> > -static inline int odp_coremask_equal(odp_coremask_t *mask1, >>> > +static inline bool odp_coremask_equal(odp_coremask_t *mask1, >>> > odp_coremask_t *mask2) >>> > { >>> > return (mask1->_u64[0] == mask2->_u64[0]); >>> > diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h >>> > index 6277a18..2e61ef6 100644 >>> > --- a/include/odp_ticketlock.h >>> > +++ b/include/odp_ticketlock.h >>> > @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>> > *ticketlock); >>> > * >>> > * @param ticketlock Ticketlock >>> > * >>> > - * @return 1 if the lock is locked, otherwise 0. >>> > + * @return true if the ticket lock is locked, otherwise false >>> > */ >>> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>> > >>> > >>> > #ifdef __cplusplus >>> > diff --git a/platform/linux-generic/odp_buffer.c >>> > b/platform/linux-generic/odp_buffer.c >>> > index 0169eec..41b6a4c 100644 >>> > --- a/platform/linux-generic/odp_buffer.c >>> > +++ b/platform/linux-generic/odp_buffer.c >>> > @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) >>> > } >>> > >>> > >>> > -int odp_buffer_is_valid(odp_buffer_t buf) >>> > +bool odp_buffer_is_valid(odp_buffer_t buf) >>> > { >>> > odp_buffer_bits_t handle; >>> > >>> > diff --git a/platform/linux-generic/odp_coremask.c >>> > b/platform/linux-generic/odp_coremask.c >>> > index c55eb72..c98a10a 100644 >>> > --- a/platform/linux-generic/odp_coremask.c >>> > +++ b/platform/linux-generic/odp_coremask.c >>> > @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t *mask) >>> > } >>> > >>> > >>> > -int odp_coremask_isset(int core, const odp_coremask_t *mask) >>> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask) >>> > { >>> > /* should not be more than 63 >>> > * core no. should be from 0..63= 64bit >>> > diff --git a/platform/linux-generic/odp_ticketlock.c >>> > b/platform/linux-generic/odp_ticketlock.c >>> > index be5b885..6cc5285 100644 >>> > --- a/platform/linux-generic/odp_ticketlock.c >>> > +++ b/platform/linux-generic/odp_ticketlock.c >>> > @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>> > *ticketlock) >>> > } >>> > >>> > >>> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>> > { >>> > return ticketlock->cur_ticket != ticketlock->next_ticket; >>> > } >>> > -- >>> > 1.9.1 >>> > >>> > >>> > _______________________________________________ >>> > lng-odp mailing list >>> > lng-odp@lists.linaro.org >>> > http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- >> Mike Holmes >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >
Just pinging for consensus. We can apply the patch if we get a number of reviewed-by replys, or we make a new patch for the ARCH doc for a page clearly spelling out our relationship to C99 and of course bool. Mike On 8 July 2014 18:28, Victor Kamensky <victor.kamensky@linaro.org> wrote: > On 8 July 2014 10:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > ODP has stated that we require C99. > > Sorry, I don't recall that. I remember that C99 compliance was discussed > and I don't have anything against it. C99 compliance does not prevent code > to be C89/C90 compliant. I.e one may state if C99 compliant client > uses ODP library it should work seamlessly. > > Have requirement for client of ODP library to be C99 compliant is > quite different > matter. Note what compliance level implementation of ODP library uses > inside > does not matter. I care about requirements to client code that would > include > ODP library header files. > > > We'd like to move towards C11 but for > > many that's a bit too aggressive. C99 is now 15 years old so that > should be > > mature enough for anyone. > > I disagree, there is code much older than that :). > > In practice old code may have its own #definition of bool. So it will be > quite messy. Although it may sort of work accidentally. Other way is to > #include <stdbool.h> gcc provides bool define there for older not C99 > compliant > code, but I suspect it could be gcc specific and still will mess up > with "other" bool definitions. > > I prefer that client visible header files would not use bool type. > > Thanks, > Victor > > > Bill > > > > > > On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes <mike.holmes@linaro.org> > wrote: > >> > >> True, but I believe we chose this as our standard, although again our > >> published arch does does not state that yet. > >> > >> A quick grep of our mailing list shows accepting patches to come into > >> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards > -std=c99 > >> compliance" > >> > >> Mike > >> > >> > >> On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> > wrote: > >>> > >>> Does not this make C99 absolute requirement for > >>> ODP library. I wonder whether it is too strict. > >>> > >>> C89/90 do not have bool type. > >>> > >>> Thanks, > >>> Victor > >>> > >>> > >>> On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: > >>> > Signed-off-by: Mike Holmes <mike.holmes@linaro.org> > >>> > --- > >>> > include/odp_buffer.h | 4 ++-- > >>> > include/odp_coremask.h | 11 ++++++++--- > >>> > include/odp_ticketlock.h | 4 ++-- > >>> > platform/linux-generic/odp_buffer.c | 2 +- > >>> > platform/linux-generic/odp_coremask.c | 2 +- > >>> > platform/linux-generic/odp_ticketlock.c | 2 +- > >>> > 6 files changed, 15 insertions(+), 10 deletions(-) > >>> > > >>> > diff --git a/include/odp_buffer.h b/include/odp_buffer.h > >>> > index d8577fd..c1c9bef 100644 > >>> > --- a/include/odp_buffer.h > >>> > +++ b/include/odp_buffer.h > >>> > @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); > >>> > * > >>> > * @param buf Buffer handle > >>> > * > >>> > - * @return 1 if valid, otherwise 0 > >>> > + * @return true if valid, otherwise false > >>> > */ > >>> > -int odp_buffer_is_valid(odp_buffer_t buf); > >>> > +bool odp_buffer_is_valid(odp_buffer_t buf); > >>> > > >>> > /** > >>> > * Print buffer metadata to STDOUT > >>> > diff --git a/include/odp_coremask.h b/include/odp_coremask.h > >>> > index 141cb6a..f5a9a0b 100644 > >>> > --- a/include/odp_coremask.h > >>> > +++ b/include/odp_coremask.h > >>> > @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t > >>> > *mask); > >>> > * Test if core is a member of mask > >>> > * @param core Core number > >>> > * @param mask Core mask to check if core num set or not > >>> > - * @return non-zero if set otherwise 0 > >>> > + * @return true if the core mask is set, otherwise false > >>> > */ > >>> > -int odp_coremask_isset(int core, const odp_coremask_t *mask); > >>> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask); > >>> > > >>> > /** > >>> > * Count number of cores in mask > >>> > @@ -163,8 +163,13 @@ static inline void > odp_coremask_xor(odp_coremask_t > >>> > *dest, odp_coremask_t *src1, > >>> > > >>> > /** > >>> > * Test if two masks contain the same cores > >>> > + * > >>> > + * @param mask1 First mask to compare > >>> > + * @param mask2 Second mask to compare > >>> > + * > >>> > + * @return true is they are equal or false otherwise > >>> > */ > >>> > -static inline int odp_coremask_equal(odp_coremask_t *mask1, > >>> > +static inline bool odp_coremask_equal(odp_coremask_t *mask1, > >>> > odp_coremask_t *mask2) > >>> > { > >>> > return (mask1->_u64[0] == mask2->_u64[0]); > >>> > diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h > >>> > index 6277a18..2e61ef6 100644 > >>> > --- a/include/odp_ticketlock.h > >>> > +++ b/include/odp_ticketlock.h > >>> > @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t > >>> > *ticketlock); > >>> > * > >>> > * @param ticketlock Ticketlock > >>> > * > >>> > - * @return 1 if the lock is locked, otherwise 0. > >>> > + * @return true if the ticket lock is locked, otherwise false > >>> > */ > >>> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); > >>> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); > >>> > > >>> > > >>> > #ifdef __cplusplus > >>> > diff --git a/platform/linux-generic/odp_buffer.c > >>> > b/platform/linux-generic/odp_buffer.c > >>> > index 0169eec..41b6a4c 100644 > >>> > --- a/platform/linux-generic/odp_buffer.c > >>> > +++ b/platform/linux-generic/odp_buffer.c > >>> > @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) > >>> > } > >>> > > >>> > > >>> > -int odp_buffer_is_valid(odp_buffer_t buf) > >>> > +bool odp_buffer_is_valid(odp_buffer_t buf) > >>> > { > >>> > odp_buffer_bits_t handle; > >>> > > >>> > diff --git a/platform/linux-generic/odp_coremask.c > >>> > b/platform/linux-generic/odp_coremask.c > >>> > index c55eb72..c98a10a 100644 > >>> > --- a/platform/linux-generic/odp_coremask.c > >>> > +++ b/platform/linux-generic/odp_coremask.c > >>> > @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t > *mask) > >>> > } > >>> > > >>> > > >>> > -int odp_coremask_isset(int core, const odp_coremask_t *mask) > >>> > +bool odp_coremask_isset(int core, const odp_coremask_t *mask) > >>> > { > >>> > /* should not be more than 63 > >>> > * core no. should be from 0..63= 64bit > >>> > diff --git a/platform/linux-generic/odp_ticketlock.c > >>> > b/platform/linux-generic/odp_ticketlock.c > >>> > index be5b885..6cc5285 100644 > >>> > --- a/platform/linux-generic/odp_ticketlock.c > >>> > +++ b/platform/linux-generic/odp_ticketlock.c > >>> > @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t > >>> > *ticketlock) > >>> > } > >>> > > >>> > > >>> > -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) > >>> > +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) > >>> > { > >>> > return ticketlock->cur_ticket != ticketlock->next_ticket; > >>> > } > >>> > -- > >>> > 1.9.1 > >>> > > >>> > > >>> > _______________________________________________ > >>> > lng-odp mailing list > >>> > lng-odp@lists.linaro.org > >>> > http://lists.linaro.org/mailman/listinfo/lng-odp > >> > >> > >> > >> > >> -- > >> Mike Holmes > >> Linaro Technical Manager / Lead > >> LNG - ODP > >> > >> _______________________________________________ > >> lng-odp mailing list > >> lng-odp@lists.linaro.org > >> http://lists.linaro.org/mailman/listinfo/lng-odp > >> > > >
On 07/09/2014 02:28 AM, Victor Kamensky wrote: > On 8 July 2014 10:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: >> ODP has stated that we require C99. > Sorry, I don't recall that. I remember that C99 compliance was discussed > and I don't have anything against it. C99 compliance does not prevent code > to be C89/C90 compliant. I.e one may state if C99 compliant client > uses ODP library it should work seamlessly. > > Have requirement for client of ODP library to be C99 compliant is > quite different > matter. Note what compliance level implementation of ODP library uses inside > does not matter. I care about requirements to client code that would include > ODP library header files. > >> We'd like to move towards C11 but for >> many that's a bit too aggressive. C99 is now 15 years old so that should be >> mature enough for anyone. > I disagree, there is code much older than that :). > > In practice old code may have its own #definition of bool. So it will be > quite messy. Although it may sort of work accidentally. Other way is to > #include <stdbool.h> it's c++ include, not C, right? > gcc provides bool define there for older not C99 compliant > code, but I suspect it could be gcc specific and still will mess up > with "other" bool definitions. > > I prefer that client visible header files would not use bool type. > > Thanks, > Victor > >> Bill >> >> >> On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes <mike.holmes@linaro.org> wrote: >>> True, but I believe we chose this as our standard, although again our >>> published arch does does not state that yet. >>> >>> A quick grep of our mailing list shows accepting patches to come into >>> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards -std=c99 >>> compliance" >>> >>> Mike >>> >>> >>> On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> wrote: >>>> Does not this make C99 absolute requirement for >>>> ODP library. I wonder whether it is too strict. >>>> >>>> C89/90 do not have bool type. >>>> >>>> Thanks, >>>> Victor >>>> >>>> >>>> On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: >>>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>>>> --- >>>>> include/odp_buffer.h | 4 ++-- >>>>> include/odp_coremask.h | 11 ++++++++--- >>>>> include/odp_ticketlock.h | 4 ++-- >>>>> platform/linux-generic/odp_buffer.c | 2 +- >>>>> platform/linux-generic/odp_coremask.c | 2 +- >>>>> platform/linux-generic/odp_ticketlock.c | 2 +- >>>>> 6 files changed, 15 insertions(+), 10 deletions(-) >>>>> >>>>> diff --git a/include/odp_buffer.h b/include/odp_buffer.h >>>>> index d8577fd..c1c9bef 100644 >>>>> --- a/include/odp_buffer.h >>>>> +++ b/include/odp_buffer.h >>>>> @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); >>>>> * >>>>> * @param buf Buffer handle >>>>> * >>>>> - * @return 1 if valid, otherwise 0 >>>>> + * @return true if valid, otherwise false >>>>> */ >>>>> -int odp_buffer_is_valid(odp_buffer_t buf); >>>>> +bool odp_buffer_is_valid(odp_buffer_t buf); >>>>> >>>>> /** >>>>> * Print buffer metadata to STDOUT >>>>> diff --git a/include/odp_coremask.h b/include/odp_coremask.h >>>>> index 141cb6a..f5a9a0b 100644 >>>>> --- a/include/odp_coremask.h >>>>> +++ b/include/odp_coremask.h >>>>> @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t >>>>> *mask); >>>>> * Test if core is a member of mask >>>>> * @param core Core number >>>>> * @param mask Core mask to check if core num set or not >>>>> - * @return non-zero if set otherwise 0 >>>>> + * @return true if the core mask is set, otherwise false >>>>> */ >>>>> -int odp_coremask_isset(int core, const odp_coremask_t *mask); >>>>> +bool odp_coremask_isset(int core, const odp_coremask_t *mask); >>>>> >>>>> /** >>>>> * Count number of cores in mask >>>>> @@ -163,8 +163,13 @@ static inline void odp_coremask_xor(odp_coremask_t >>>>> *dest, odp_coremask_t *src1, >>>>> >>>>> /** >>>>> * Test if two masks contain the same cores >>>>> + * >>>>> + * @param mask1 First mask to compare >>>>> + * @param mask2 Second mask to compare >>>>> + * >>>>> + * @return true is they are equal or false otherwise >>>>> */ >>>>> -static inline int odp_coremask_equal(odp_coremask_t *mask1, >>>>> +static inline bool odp_coremask_equal(odp_coremask_t *mask1, >>>>> odp_coremask_t *mask2) >>>>> { >>>>> return (mask1->_u64[0] == mask2->_u64[0]); >>>>> diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h >>>>> index 6277a18..2e61ef6 100644 >>>>> --- a/include/odp_ticketlock.h >>>>> +++ b/include/odp_ticketlock.h >>>>> @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>>>> *ticketlock); >>>>> * >>>>> * @param ticketlock Ticketlock >>>>> * >>>>> - * @return 1 if the lock is locked, otherwise 0. >>>>> + * @return true if the ticket lock is locked, otherwise false >>>>> */ >>>>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>>>> +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>>>> >>>>> >>>>> #ifdef __cplusplus >>>>> diff --git a/platform/linux-generic/odp_buffer.c >>>>> b/platform/linux-generic/odp_buffer.c >>>>> index 0169eec..41b6a4c 100644 >>>>> --- a/platform/linux-generic/odp_buffer.c >>>>> +++ b/platform/linux-generic/odp_buffer.c >>>>> @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) >>>>> } >>>>> >>>>> >>>>> -int odp_buffer_is_valid(odp_buffer_t buf) >>>>> +bool odp_buffer_is_valid(odp_buffer_t buf) >>>>> { >>>>> odp_buffer_bits_t handle; >>>>> >>>>> diff --git a/platform/linux-generic/odp_coremask.c >>>>> b/platform/linux-generic/odp_coremask.c >>>>> index c55eb72..c98a10a 100644 >>>>> --- a/platform/linux-generic/odp_coremask.c >>>>> +++ b/platform/linux-generic/odp_coremask.c >>>>> @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t *mask) >>>>> } >>>>> >>>>> >>>>> -int odp_coremask_isset(int core, const odp_coremask_t *mask) >>>>> +bool odp_coremask_isset(int core, const odp_coremask_t *mask) >>>>> { >>>>> /* should not be more than 63 >>>>> * core no. should be from 0..63= 64bit >>>>> diff --git a/platform/linux-generic/odp_ticketlock.c >>>>> b/platform/linux-generic/odp_ticketlock.c >>>>> index be5b885..6cc5285 100644 >>>>> --- a/platform/linux-generic/odp_ticketlock.c >>>>> +++ b/platform/linux-generic/odp_ticketlock.c >>>>> @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>>>> *ticketlock) >>>>> } >>>>> >>>>> >>>>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>>>> +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>>>> { >>>>> return ticketlock->cur_ticket != ticketlock->next_ticket; >>>>> } >>>>> -- >>>>> 1.9.1 >>>>> >>>>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> lng-odp@lists.linaro.org >>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >>> >>> >>> -- >>> Mike Holmes >>> Linaro Technical Manager / Lead >>> LNG - ODP >>> >>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp
I don't think is is restricted to C++ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf I may have the wrong doc but bool is part of C99 if the above is correct I think. On 9 July 2014 14:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > On 07/09/2014 02:28 AM, Victor Kamensky wrote: > >> On 8 July 2014 10:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: >> >>> ODP has stated that we require C99. >>> >> Sorry, I don't recall that. I remember that C99 compliance was discussed >> and I don't have anything against it. C99 compliance does not prevent code >> to be C89/C90 compliant. I.e one may state if C99 compliant client >> uses ODP library it should work seamlessly. >> >> Have requirement for client of ODP library to be C99 compliant is >> quite different >> matter. Note what compliance level implementation of ODP library uses >> inside >> does not matter. I care about requirements to client code that would >> include >> ODP library header files. >> >> We'd like to move towards C11 but for >>> many that's a bit too aggressive. C99 is now 15 years old so that >>> should be >>> mature enough for anyone. >>> >> I disagree, there is code much older than that :). >> >> In practice old code may have its own #definition of bool. So it will be >> quite messy. Although it may sort of work accidentally. Other way is to >> #include <stdbool.h> >> > > it's c++ include, not C, right? > > > gcc provides bool define there for older not C99 compliant >> code, but I suspect it could be gcc specific and still will mess up >> with "other" bool definitions. >> >> I prefer that client visible header files would not use bool type. >> >> Thanks, >> Victor >> >> Bill >>> >>> >>> On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes <mike.holmes@linaro.org> >>> wrote: >>> >>>> True, but I believe we chose this as our standard, although again our >>>> published arch does does not state that yet. >>>> >>>> A quick grep of our mailing list shows accepting patches to come into >>>> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards >>>> -std=c99 >>>> compliance" >>>> >>>> Mike >>>> >>>> >>>> On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> >>>> wrote: >>>> >>>>> Does not this make C99 absolute requirement for >>>>> ODP library. I wonder whether it is too strict. >>>>> >>>>> C89/90 do not have bool type. >>>>> >>>>> Thanks, >>>>> Victor >>>>> >>>>> >>>>> On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: >>>>> >>>>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>>>>> --- >>>>>> include/odp_buffer.h | 4 ++-- >>>>>> include/odp_coremask.h | 11 ++++++++--- >>>>>> include/odp_ticketlock.h | 4 ++-- >>>>>> platform/linux-generic/odp_buffer.c | 2 +- >>>>>> platform/linux-generic/odp_coremask.c | 2 +- >>>>>> platform/linux-generic/odp_ticketlock.c | 2 +- >>>>>> 6 files changed, 15 insertions(+), 10 deletions(-) >>>>>> >>>>>> diff --git a/include/odp_buffer.h b/include/odp_buffer.h >>>>>> index d8577fd..c1c9bef 100644 >>>>>> --- a/include/odp_buffer.h >>>>>> +++ b/include/odp_buffer.h >>>>>> @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); >>>>>> * >>>>>> * @param buf Buffer handle >>>>>> * >>>>>> - * @return 1 if valid, otherwise 0 >>>>>> + * @return true if valid, otherwise false >>>>>> */ >>>>>> -int odp_buffer_is_valid(odp_buffer_t buf); >>>>>> +bool odp_buffer_is_valid(odp_buffer_t buf); >>>>>> >>>>>> /** >>>>>> * Print buffer metadata to STDOUT >>>>>> diff --git a/include/odp_coremask.h b/include/odp_coremask.h >>>>>> index 141cb6a..f5a9a0b 100644 >>>>>> --- a/include/odp_coremask.h >>>>>> +++ b/include/odp_coremask.h >>>>>> @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t >>>>>> *mask); >>>>>> * Test if core is a member of mask >>>>>> * @param core Core number >>>>>> * @param mask Core mask to check if core num set or not >>>>>> - * @return non-zero if set otherwise 0 >>>>>> + * @return true if the core mask is set, otherwise false >>>>>> */ >>>>>> -int odp_coremask_isset(int core, const odp_coremask_t *mask); >>>>>> +bool odp_coremask_isset(int core, const odp_coremask_t *mask); >>>>>> >>>>>> /** >>>>>> * Count number of cores in mask >>>>>> @@ -163,8 +163,13 @@ static inline void odp_coremask_xor(odp_coremask_ >>>>>> t >>>>>> *dest, odp_coremask_t *src1, >>>>>> >>>>>> /** >>>>>> * Test if two masks contain the same cores >>>>>> + * >>>>>> + * @param mask1 First mask to compare >>>>>> + * @param mask2 Second mask to compare >>>>>> + * >>>>>> + * @return true is they are equal or false otherwise >>>>>> */ >>>>>> -static inline int odp_coremask_equal(odp_coremask_t *mask1, >>>>>> +static inline bool odp_coremask_equal(odp_coremask_t *mask1, >>>>>> odp_coremask_t *mask2) >>>>>> { >>>>>> return (mask1->_u64[0] == mask2->_u64[0]); >>>>>> diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h >>>>>> index 6277a18..2e61ef6 100644 >>>>>> --- a/include/odp_ticketlock.h >>>>>> +++ b/include/odp_ticketlock.h >>>>>> @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>>>>> *ticketlock); >>>>>> * >>>>>> * @param ticketlock Ticketlock >>>>>> * >>>>>> - * @return 1 if the lock is locked, otherwise 0. >>>>>> + * @return true if the ticket lock is locked, otherwise false >>>>>> */ >>>>>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>>>>> +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>>>>> >>>>>> >>>>>> #ifdef __cplusplus >>>>>> diff --git a/platform/linux-generic/odp_buffer.c >>>>>> b/platform/linux-generic/odp_buffer.c >>>>>> index 0169eec..41b6a4c 100644 >>>>>> --- a/platform/linux-generic/odp_buffer.c >>>>>> +++ b/platform/linux-generic/odp_buffer.c >>>>>> @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) >>>>>> } >>>>>> >>>>>> >>>>>> -int odp_buffer_is_valid(odp_buffer_t buf) >>>>>> +bool odp_buffer_is_valid(odp_buffer_t buf) >>>>>> { >>>>>> odp_buffer_bits_t handle; >>>>>> >>>>>> diff --git a/platform/linux-generic/odp_coremask.c >>>>>> b/platform/linux-generic/odp_coremask.c >>>>>> index c55eb72..c98a10a 100644 >>>>>> --- a/platform/linux-generic/odp_coremask.c >>>>>> +++ b/platform/linux-generic/odp_coremask.c >>>>>> @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t >>>>>> *mask) >>>>>> } >>>>>> >>>>>> >>>>>> -int odp_coremask_isset(int core, const odp_coremask_t *mask) >>>>>> +bool odp_coremask_isset(int core, const odp_coremask_t *mask) >>>>>> { >>>>>> /* should not be more than 63 >>>>>> * core no. should be from 0..63= 64bit >>>>>> diff --git a/platform/linux-generic/odp_ticketlock.c >>>>>> b/platform/linux-generic/odp_ticketlock.c >>>>>> index be5b885..6cc5285 100644 >>>>>> --- a/platform/linux-generic/odp_ticketlock.c >>>>>> +++ b/platform/linux-generic/odp_ticketlock.c >>>>>> @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>>>>> *ticketlock) >>>>>> } >>>>>> >>>>>> >>>>>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>>>>> +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>>>>> { >>>>>> return ticketlock->cur_ticket != ticketlock->next_ticket; >>>>>> } >>>>>> -- >>>>>> 1.9.1 >>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> lng-odp mailing list >>>>>> lng-odp@lists.linaro.org >>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>> >>>>> >>>> >>>> >>>> -- >>>> Mike Holmes >>>> Linaro Technical Manager / Lead >>>> LNG - ODP >>>> >>>> _______________________________________________ >>>> lng-odp mailing list >>>> lng-odp@lists.linaro.org >>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>> >>>> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
A bit of research shows that the C99 native type is actually _Bool and these hold the value 0 and 1. By contrast, the type 'bool' as well as 'true' and 'false' are macros defined in <stdbool.h> that map to these. See http://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdbool.h.html for details. Bill On Wed, Jul 9, 2014 at 2:05 PM, Mike Holmes <mike.holmes@linaro.org> wrote: > I don't think is is restricted to C++ > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf > > I may have the wrong doc but bool is part of C99 if the above is correct I > think. > > > On 9 July 2014 14:57, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > >> On 07/09/2014 02:28 AM, Victor Kamensky wrote: >> >>> On 8 July 2014 10:27, Bill Fischofer <bill.fischofer@linaro.org> wrote: >>> >>>> ODP has stated that we require C99. >>>> >>> Sorry, I don't recall that. I remember that C99 compliance was discussed >>> and I don't have anything against it. C99 compliance does not prevent >>> code >>> to be C89/C90 compliant. I.e one may state if C99 compliant client >>> uses ODP library it should work seamlessly. >>> >>> Have requirement for client of ODP library to be C99 compliant is >>> quite different >>> matter. Note what compliance level implementation of ODP library uses >>> inside >>> does not matter. I care about requirements to client code that would >>> include >>> ODP library header files. >>> >>> We'd like to move towards C11 but for >>>> many that's a bit too aggressive. C99 is now 15 years old so that >>>> should be >>>> mature enough for anyone. >>>> >>> I disagree, there is code much older than that :). >>> >>> In practice old code may have its own #definition of bool. So it will be >>> quite messy. Although it may sort of work accidentally. Other way is to >>> #include <stdbool.h> >>> >> >> it's c++ include, not C, right? >> >> >> gcc provides bool define there for older not C99 compliant >>> code, but I suspect it could be gcc specific and still will mess up >>> with "other" bool definitions. >>> >>> I prefer that client visible header files would not use bool type. >>> >>> Thanks, >>> Victor >>> >>> Bill >>>> >>>> >>>> On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes <mike.holmes@linaro.org> >>>> wrote: >>>> >>>>> True, but I believe we chose this as our standard, although again our >>>>> published arch does does not state that yet. >>>>> >>>>> A quick grep of our mailing list shows accepting patches to come into >>>>> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] patches towards >>>>> -std=c99 >>>>> compliance" >>>>> >>>>> Mike >>>>> >>>>> >>>>> On 8 July 2014 12:58, Victor Kamensky <victor.kamensky@linaro.org> >>>>> wrote: >>>>> >>>>>> Does not this make C99 absolute requirement for >>>>>> ODP library. I wonder whether it is too strict. >>>>>> >>>>>> C89/90 do not have bool type. >>>>>> >>>>>> Thanks, >>>>>> Victor >>>>>> >>>>>> >>>>>> On 8 July 2014 09:48, Mike Holmes <mike.holmes@linaro.org> wrote: >>>>>> >>>>>>> Signed-off-by: Mike Holmes <mike.holmes@linaro.org> >>>>>>> --- >>>>>>> include/odp_buffer.h | 4 ++-- >>>>>>> include/odp_coremask.h | 11 ++++++++--- >>>>>>> include/odp_ticketlock.h | 4 ++-- >>>>>>> platform/linux-generic/odp_buffer.c | 2 +- >>>>>>> platform/linux-generic/odp_coremask.c | 2 +- >>>>>>> platform/linux-generic/odp_ticketlock.c | 2 +- >>>>>>> 6 files changed, 15 insertions(+), 10 deletions(-) >>>>>>> >>>>>>> diff --git a/include/odp_buffer.h b/include/odp_buffer.h >>>>>>> index d8577fd..c1c9bef 100644 >>>>>>> --- a/include/odp_buffer.h >>>>>>> +++ b/include/odp_buffer.h >>>>>>> @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); >>>>>>> * >>>>>>> * @param buf Buffer handle >>>>>>> * >>>>>>> - * @return 1 if valid, otherwise 0 >>>>>>> + * @return true if valid, otherwise false >>>>>>> */ >>>>>>> -int odp_buffer_is_valid(odp_buffer_t buf); >>>>>>> +bool odp_buffer_is_valid(odp_buffer_t buf); >>>>>>> >>>>>>> /** >>>>>>> * Print buffer metadata to STDOUT >>>>>>> diff --git a/include/odp_coremask.h b/include/odp_coremask.h >>>>>>> index 141cb6a..f5a9a0b 100644 >>>>>>> --- a/include/odp_coremask.h >>>>>>> +++ b/include/odp_coremask.h >>>>>>> @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t >>>>>>> *mask); >>>>>>> * Test if core is a member of mask >>>>>>> * @param core Core number >>>>>>> * @param mask Core mask to check if core num set or not >>>>>>> - * @return non-zero if set otherwise 0 >>>>>>> + * @return true if the core mask is set, otherwise false >>>>>>> */ >>>>>>> -int odp_coremask_isset(int core, const odp_coremask_t *mask); >>>>>>> +bool odp_coremask_isset(int core, const odp_coremask_t *mask); >>>>>>> >>>>>>> /** >>>>>>> * Count number of cores in mask >>>>>>> @@ -163,8 +163,13 @@ static inline void >>>>>>> odp_coremask_xor(odp_coremask_t >>>>>>> *dest, odp_coremask_t *src1, >>>>>>> >>>>>>> /** >>>>>>> * Test if two masks contain the same cores >>>>>>> + * >>>>>>> + * @param mask1 First mask to compare >>>>>>> + * @param mask2 Second mask to compare >>>>>>> + * >>>>>>> + * @return true is they are equal or false otherwise >>>>>>> */ >>>>>>> -static inline int odp_coremask_equal(odp_coremask_t *mask1, >>>>>>> +static inline bool odp_coremask_equal(odp_coremask_t *mask1, >>>>>>> odp_coremask_t *mask2) >>>>>>> { >>>>>>> return (mask1->_u64[0] == mask2->_u64[0]); >>>>>>> diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h >>>>>>> index 6277a18..2e61ef6 100644 >>>>>>> --- a/include/odp_ticketlock.h >>>>>>> +++ b/include/odp_ticketlock.h >>>>>>> @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>>>>>> *ticketlock); >>>>>>> * >>>>>>> * @param ticketlock Ticketlock >>>>>>> * >>>>>>> - * @return 1 if the lock is locked, otherwise 0. >>>>>>> + * @return true if the ticket lock is locked, otherwise false >>>>>>> */ >>>>>>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>>>>>> +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); >>>>>>> >>>>>>> >>>>>>> #ifdef __cplusplus >>>>>>> diff --git a/platform/linux-generic/odp_buffer.c >>>>>>> b/platform/linux-generic/odp_buffer.c >>>>>>> index 0169eec..41b6a4c 100644 >>>>>>> --- a/platform/linux-generic/odp_buffer.c >>>>>>> +++ b/platform/linux-generic/odp_buffer.c >>>>>>> @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) >>>>>>> } >>>>>>> >>>>>>> >>>>>>> -int odp_buffer_is_valid(odp_buffer_t buf) >>>>>>> +bool odp_buffer_is_valid(odp_buffer_t buf) >>>>>>> { >>>>>>> odp_buffer_bits_t handle; >>>>>>> >>>>>>> diff --git a/platform/linux-generic/odp_coremask.c >>>>>>> b/platform/linux-generic/odp_coremask.c >>>>>>> index c55eb72..c98a10a 100644 >>>>>>> --- a/platform/linux-generic/odp_coremask.c >>>>>>> +++ b/platform/linux-generic/odp_coremask.c >>>>>>> @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t >>>>>>> *mask) >>>>>>> } >>>>>>> >>>>>>> >>>>>>> -int odp_coremask_isset(int core, const odp_coremask_t *mask) >>>>>>> +bool odp_coremask_isset(int core, const odp_coremask_t *mask) >>>>>>> { >>>>>>> /* should not be more than 63 >>>>>>> * core no. should be from 0..63= 64bit >>>>>>> diff --git a/platform/linux-generic/odp_ticketlock.c >>>>>>> b/platform/linux-generic/odp_ticketlock.c >>>>>>> index be5b885..6cc5285 100644 >>>>>>> --- a/platform/linux-generic/odp_ticketlock.c >>>>>>> +++ b/platform/linux-generic/odp_ticketlock.c >>>>>>> @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t >>>>>>> *ticketlock) >>>>>>> } >>>>>>> >>>>>>> >>>>>>> -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>>>>>> +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) >>>>>>> { >>>>>>> return ticketlock->cur_ticket != ticketlock->next_ticket; >>>>>>> } >>>>>>> -- >>>>>>> 1.9.1 >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> lng-odp mailing list >>>>>>> lng-odp@lists.linaro.org >>>>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Mike Holmes >>>>> Linaro Technical Manager / Lead >>>>> LNG - ODP >>>>> >>>>> _______________________________________________ >>>>> lng-odp mailing list >>>>> lng-odp@lists.linaro.org >>>>> http://lists.linaro.org/mailman/listinfo/lng-odp >>>>> >>>>> _______________________________________________ >>> lng-odp mailing list >>> lng-odp@lists.linaro.org >>> http://lists.linaro.org/mailman/listinfo/lng-odp >>> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> http://lists.linaro.org/mailman/listinfo/lng-odp >> > > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp > >
What is faster bool or int? Just serious :) For init function bool is ok, it might go to one byte. But for regular per-packet/per-buffer checks? What is preferred to cmp, jal and other branch instruction? For some reason linux kernel does not have bool for check functions. For that might be some reason. Quick test shows that int is faster: #include <stdio.h> #include <stdbool.h> int main() { unsigned long long i,j; #ifdef TEST_BOOL bool z=0,x=0,c=0; #warning "bool version" #else int z=0,x=0,c=0; #warning "int version" #endif for (i = 0; i < 10000000000; i++) z = i & 1; for (j = 0; i < 1000000000; i++) { c = j & 1; if ((c + z) > 0) x = 1; else x = c & 1; } printf("z %d x %d c %d\n", z, x, c); } maxim@maxim-lap:/tmp/test$ time ./test z 1 x 0 c 0 real 0m32.743s user 0m32.712s sys 0m0.000s maxim@maxim-lap:/tmp/test$ time ./test_int z 1 x 0 c 0 real 0m31.971s user 0m31.944s sys 0m0.000s BR, Maxim. On 07/09/2014 11:10 PM, Bill Fischofer wrote: > A bit of research shows that the C99 native type is actually _Bool and > these hold the value 0 and 1. By contrast, the type 'bool' as well as > 'true' and 'false' are macros defined in <stdbool.h> that map to > these. See > http://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdbool.h.html > for details. > > Bill > > > On Wed, Jul 9, 2014 at 2:05 PM, Mike Holmes <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> wrote: > > I don't think is is restricted to C++ > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf > > I may have the wrong doc but bool is part of C99 if the above is > correct I think. > > > On 9 July 2014 14:57, Maxim Uvarov <maxim.uvarov@linaro.org > <mailto:maxim.uvarov@linaro.org>> wrote: > > On 07/09/2014 02:28 AM, Victor Kamensky wrote: > > On 8 July 2014 10:27, Bill Fischofer > <bill.fischofer@linaro.org > <mailto:bill.fischofer@linaro.org>> wrote: > > ODP has stated that we require C99. > > Sorry, I don't recall that. I remember that C99 compliance > was discussed > and I don't have anything against it. C99 compliance does > not prevent code > to be C89/C90 compliant. I.e one may state if C99 > compliant client > uses ODP library it should work seamlessly. > > Have requirement for client of ODP library to be C99 > compliant is > quite different > matter. Note what compliance level implementation of ODP > library uses inside > does not matter. I care about requirements to client code > that would include > ODP library header files. > > We'd like to move towards C11 but for > many that's a bit too aggressive. C99 is now 15 years > old so that should be > mature enough for anyone. > > I disagree, there is code much older than that :). > > In practice old code may have its own #definition of bool. > So it will be > quite messy. Although it may sort of work accidentally. > Other way is to > #include <stdbool.h> > > > it's c++ include, not C, right? > > > gcc provides bool define there for older not C99 compliant > code, but I suspect it could be gcc specific and still > will mess up > with "other" bool definitions. > > I prefer that client visible header files would not use > bool type. > > Thanks, > Victor > > Bill > > > On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes > <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> wrote: > > True, but I believe we chose this as our standard, > although again our > published arch does does not state that yet. > > A quick grep of our mailing list shows accepting > patches to come into > compliance with C99 - see "[lng-odp] [PATCHv2 0/4] > patches towards -std=c99 > compliance" > > Mike > > > On 8 July 2014 12:58, Victor Kamensky > <victor.kamensky@linaro.org > <mailto:victor.kamensky@linaro.org>> wrote: > > Does not this make C99 absolute requirement for > ODP library. I wonder whether it is too strict. > > C89/90 do not have bool type. > > Thanks, > Victor > > > On 8 July 2014 09:48, Mike Holmes > <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> wrote: > > Signed-off-by: Mike Holmes > <mike.holmes@linaro.org > <mailto:mike.holmes@linaro.org>> > --- > include/odp_buffer.h | 4 ++-- > include/odp_coremask.h | 11 ++++++++--- > include/odp_ticketlock.h | 4 ++-- > platform/linux-generic/odp_buffer.c > | 2 +- > platform/linux-generic/odp_coremask.c > | 2 +- > platform/linux-generic/odp_ticketlock.c > | 2 +- > 6 files changed, 15 insertions(+), 10 > deletions(-) > > diff --git a/include/odp_buffer.h > b/include/odp_buffer.h > index d8577fd..c1c9bef 100644 > --- a/include/odp_buffer.h > +++ b/include/odp_buffer.h > @@ -71,9 +71,9 @@ int > odp_buffer_type(odp_buffer_t buf); > * > * @param buf Buffer handle > * > - * @return 1 if valid, otherwise 0 > + * @return true if valid, otherwise false > */ > -int odp_buffer_is_valid(odp_buffer_t buf); > +bool odp_buffer_is_valid(odp_buffer_t buf); > > /** > * Print buffer metadata to STDOUT > diff --git a/include/odp_coremask.h > b/include/odp_coremask.h > index 141cb6a..f5a9a0b 100644 > --- a/include/odp_coremask.h > +++ b/include/odp_coremask.h > @@ -109,9 +109,9 @@ void > odp_coremask_clr(int core, odp_coremask_t > *mask); > * Test if core is a member of mask > * @param core Core number > * @param mask Core mask to check if > core num set or not > - * @return non-zero if set otherwise 0 > + * @return true if the core mask is > set, otherwise false > */ > -int odp_coremask_isset(int core, const > odp_coremask_t *mask); > +bool odp_coremask_isset(int core, const > odp_coremask_t *mask); > > /** > * Count number of cores in mask > @@ -163,8 +163,13 @@ static inline void > odp_coremask_xor(odp_coremask_t > *dest, odp_coremask_t *src1, > > /** > * Test if two masks contain the same cores > + * > + * @param mask1 First mask to compare > + * @param mask2 Second mask to compare > + * > + * @return true is they are equal or > false otherwise > */ > -static inline int > odp_coremask_equal(odp_coremask_t *mask1, > +static inline bool > odp_coremask_equal(odp_coremask_t *mask1, > odp_coremask_t *mask2) > { > return (mask1->_u64[0] == > mask2->_u64[0]); > diff --git a/include/odp_ticketlock.h > b/include/odp_ticketlock.h > index 6277a18..2e61ef6 100644 > --- a/include/odp_ticketlock.h > +++ b/include/odp_ticketlock.h > @@ -71,9 +71,9 @@ void > odp_ticketlock_unlock(odp_ticketlock_t > *ticketlock); > * > * @param ticketlock Ticketlock > * > - * @return 1 if the lock is locked, > otherwise 0. > + * @return true if the ticket lock is > locked, otherwise false > */ > -int > odp_ticketlock_is_locked(odp_ticketlock_t > *ticketlock); > +bool > odp_ticketlock_is_locked(odp_ticketlock_t > *ticketlock); > > > #ifdef __cplusplus > diff --git > a/platform/linux-generic/odp_buffer.c > b/platform/linux-generic/odp_buffer.c > index 0169eec..41b6a4c 100644 > --- a/platform/linux-generic/odp_buffer.c > +++ b/platform/linux-generic/odp_buffer.c > @@ -36,7 +36,7 @@ int > odp_buffer_type(odp_buffer_t buf) > } > > > -int odp_buffer_is_valid(odp_buffer_t buf) > +bool odp_buffer_is_valid(odp_buffer_t buf) > { > odp_buffer_bits_t handle; > > diff --git > a/platform/linux-generic/odp_coremask.c > b/platform/linux-generic/odp_coremask.c > index c55eb72..c98a10a 100644 > --- a/platform/linux-generic/odp_coremask.c > +++ b/platform/linux-generic/odp_coremask.c > @@ -81,7 +81,7 @@ void > odp_coremask_clr(int core, odp_coremask_t > *mask) > } > > > -int odp_coremask_isset(int core, const > odp_coremask_t *mask) > +bool odp_coremask_isset(int core, const > odp_coremask_t *mask) > { > /* should not be more than 63 > * core no. should be from 0..63= > 64bit > diff --git > a/platform/linux-generic/odp_ticketlock.c > b/platform/linux-generic/odp_ticketlock.c > index be5b885..6cc5285 100644 > --- a/platform/linux-generic/odp_ticketlock.c > +++ b/platform/linux-generic/odp_ticketlock.c > @@ -45,7 +45,7 @@ void > odp_ticketlock_unlock(odp_ticketlock_t > *ticketlock) > } > > > -int > odp_ticketlock_is_locked(odp_ticketlock_t > *ticketlock) > +bool > odp_ticketlock_is_locked(odp_ticketlock_t > *ticketlock) > { > return ticketlock->cur_ticket != > ticketlock->next_ticket; > } > -- > 1.9.1 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > Mike Holmes > Linaro Technical Manager / Lead > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > > > > > -- > *Mike Holmes* > Linaro Technical Manager / Lead > LNG - ODP > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> > http://lists.linaro.org/mailman/listinfo/lng-odp > >
I suspect _Bool is implemented at int in most compilers. If you're using structs, you'd use the bit notation (int flag : 1;) rather than _Bool anyway. Sounds like given Victor's concerns we should probably just stick with the simpler and universal int? Bill On Wed, Jul 9, 2014 at 2:19 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > What is faster bool or int? Just serious :) > > For init function bool is ok, it might go to one byte. But for regular > per-packet/per-buffer checks? What is preferred to cmp, jal and other > branch instruction? > For some reason linux kernel does not have bool for check functions. For > that might be some reason. > > Quick test shows that int is faster: > #include <stdio.h> > #include <stdbool.h> > > int main() > > { > unsigned long long i,j; > > #ifdef TEST_BOOL > bool z=0,x=0,c=0; > #warning "bool version" > #else > int z=0,x=0,c=0; > #warning "int version" > #endif > for (i = 0; i < 10000000000; i++) > z = i & 1; > for (j = 0; i < 1000000000; i++) { > c = j & 1; > if ((c + z) > 0) > x = 1; > else > x = c & 1; > } > > printf("z %d x %d c %d\n", z, x, c); > } > > > maxim@maxim-lap:/tmp/test$ time ./test > z 1 x 0 c 0 > > real 0m32.743s > user 0m32.712s > sys 0m0.000s > maxim@maxim-lap:/tmp/test$ time ./test_int > z 1 x 0 c 0 > > real 0m31.971s > user 0m31.944s > sys 0m0.000s > > BR, > Maxim. > > > On 07/09/2014 11:10 PM, Bill Fischofer wrote: > >> A bit of research shows that the C99 native type is actually _Bool and >> these hold the value 0 and 1. By contrast, the type 'bool' as well as >> 'true' and 'false' are macros defined in <stdbool.h> that map to these. >> See http://pubs.opengroup.org/onlinepubs/009695399/basedefs/ >> stdbool.h.html for details. >> >> Bill >> >> >> On Wed, Jul 9, 2014 at 2:05 PM, Mike Holmes <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> wrote: >> >> I don't think is is restricted to C++ >> >> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf >> >> I may have the wrong doc but bool is part of C99 if the above is >> correct I think. >> >> >> On 9 July 2014 14:57, Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> wrote: >> >> On 07/09/2014 02:28 AM, Victor Kamensky wrote: >> >> On 8 July 2014 10:27, Bill Fischofer >> <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> wrote: >> >> ODP has stated that we require C99. >> >> Sorry, I don't recall that. I remember that C99 compliance >> was discussed >> and I don't have anything against it. C99 compliance does >> not prevent code >> to be C89/C90 compliant. I.e one may state if C99 >> compliant client >> uses ODP library it should work seamlessly. >> >> Have requirement for client of ODP library to be C99 >> compliant is >> quite different >> matter. Note what compliance level implementation of ODP >> library uses inside >> does not matter. I care about requirements to client code >> that would include >> ODP library header files. >> >> We'd like to move towards C11 but for >> many that's a bit too aggressive. C99 is now 15 years >> old so that should be >> mature enough for anyone. >> >> I disagree, there is code much older than that :). >> >> In practice old code may have its own #definition of bool. >> So it will be >> quite messy. Although it may sort of work accidentally. >> Other way is to >> #include <stdbool.h> >> >> >> it's c++ include, not C, right? >> >> >> gcc provides bool define there for older not C99 compliant >> code, but I suspect it could be gcc specific and still >> will mess up >> with "other" bool definitions. >> >> I prefer that client visible header files would not use >> bool type. >> >> Thanks, >> Victor >> >> Bill >> >> >> On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes >> <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> wrote: >> >> True, but I believe we chose this as our standard, >> although again our >> published arch does does not state that yet. >> >> A quick grep of our mailing list shows accepting >> patches to come into >> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] >> patches towards -std=c99 >> compliance" >> >> Mike >> >> >> On 8 July 2014 12:58, Victor Kamensky >> <victor.kamensky@linaro.org >> <mailto:victor.kamensky@linaro.org>> wrote: >> >> Does not this make C99 absolute requirement for >> ODP library. I wonder whether it is too strict. >> >> C89/90 do not have bool type. >> >> Thanks, >> Victor >> >> >> On 8 July 2014 09:48, Mike Holmes >> <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> wrote: >> >> Signed-off-by: Mike Holmes >> <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> >> >> --- >> include/odp_buffer.h | 4 ++-- >> include/odp_coremask.h | 11 ++++++++--- >> include/odp_ticketlock.h | 4 ++-- >> platform/linux-generic/odp_buffer.c >> | 2 +- >> platform/linux-generic/odp_coremask.c >> | 2 +- >> platform/linux-generic/odp_ticketlock.c >> | 2 +- >> 6 files changed, 15 insertions(+), 10 >> deletions(-) >> >> diff --git a/include/odp_buffer.h >> b/include/odp_buffer.h >> index d8577fd..c1c9bef 100644 >> --- a/include/odp_buffer.h >> +++ b/include/odp_buffer.h >> @@ -71,9 +71,9 @@ int >> odp_buffer_type(odp_buffer_t buf); >> * >> * @param buf Buffer handle >> * >> - * @return 1 if valid, otherwise 0 >> + * @return true if valid, otherwise false >> */ >> -int odp_buffer_is_valid(odp_buffer_t buf); >> +bool odp_buffer_is_valid(odp_buffer_t buf); >> >> /** >> * Print buffer metadata to STDOUT >> diff --git a/include/odp_coremask.h >> b/include/odp_coremask.h >> index 141cb6a..f5a9a0b 100644 >> --- a/include/odp_coremask.h >> +++ b/include/odp_coremask.h >> @@ -109,9 +109,9 @@ void >> odp_coremask_clr(int core, odp_coremask_t >> *mask); >> * Test if core is a member of mask >> * @param core Core number >> * @param mask Core mask to check if >> core num set or not >> - * @return non-zero if set otherwise 0 >> + * @return true if the core mask is >> set, otherwise false >> */ >> -int odp_coremask_isset(int core, const >> odp_coremask_t *mask); >> +bool odp_coremask_isset(int core, const >> odp_coremask_t *mask); >> >> /** >> * Count number of cores in mask >> @@ -163,8 +163,13 @@ static inline void >> odp_coremask_xor(odp_coremask_t >> *dest, odp_coremask_t *src1, >> >> /** >> * Test if two masks contain the same cores >> + * >> + * @param mask1 First mask to compare >> + * @param mask2 Second mask to compare >> + * >> + * @return true is they are equal or >> false otherwise >> */ >> -static inline int >> odp_coremask_equal(odp_coremask_t *mask1, >> +static inline bool >> odp_coremask_equal(odp_coremask_t *mask1, >> odp_coremask_t *mask2) >> { >> return (mask1->_u64[0] == >> mask2->_u64[0]); >> diff --git a/include/odp_ticketlock.h >> b/include/odp_ticketlock.h >> index 6277a18..2e61ef6 100644 >> --- a/include/odp_ticketlock.h >> +++ b/include/odp_ticketlock.h >> @@ -71,9 +71,9 @@ void >> odp_ticketlock_unlock(odp_ticketlock_t >> *ticketlock); >> * >> * @param ticketlock Ticketlock >> * >> - * @return 1 if the lock is locked, >> otherwise 0. >> + * @return true if the ticket lock is >> locked, otherwise false >> */ >> -int >> odp_ticketlock_is_locked(odp_ticketlock_t >> *ticketlock); >> +bool >> odp_ticketlock_is_locked(odp_ticketlock_t >> *ticketlock); >> >> >> #ifdef __cplusplus >> diff --git >> a/platform/linux-generic/odp_buffer.c >> b/platform/linux-generic/odp_buffer.c >> index 0169eec..41b6a4c 100644 >> --- a/platform/linux-generic/odp_buffer.c >> +++ b/platform/linux-generic/odp_buffer.c >> @@ -36,7 +36,7 @@ int >> odp_buffer_type(odp_buffer_t buf) >> } >> >> >> -int odp_buffer_is_valid(odp_buffer_t buf) >> +bool odp_buffer_is_valid(odp_buffer_t buf) >> { >> odp_buffer_bits_t handle; >> >> diff --git >> a/platform/linux-generic/odp_coremask.c >> b/platform/linux-generic/odp_coremask.c >> index c55eb72..c98a10a 100644 >> --- a/platform/linux-generic/odp_coremask.c >> +++ b/platform/linux-generic/odp_coremask.c >> @@ -81,7 +81,7 @@ void >> odp_coremask_clr(int core, odp_coremask_t >> *mask) >> } >> >> >> -int odp_coremask_isset(int core, const >> odp_coremask_t *mask) >> +bool odp_coremask_isset(int core, const >> odp_coremask_t *mask) >> { >> /* should not be more than 63 >> * core no. should be from 0..63= >> 64bit >> diff --git >> a/platform/linux-generic/odp_ticketlock.c >> b/platform/linux-generic/odp_ticketlock.c >> index be5b885..6cc5285 100644 >> --- a/platform/linux-generic/odp_ticketlock.c >> +++ b/platform/linux-generic/odp_ticketlock.c >> @@ -45,7 +45,7 @@ void >> odp_ticketlock_unlock(odp_ticketlock_t >> *ticketlock) >> } >> >> >> -int >> odp_ticketlock_is_locked(odp_ticketlock_t >> *ticketlock) >> +bool >> odp_ticketlock_is_locked(odp_ticketlock_t >> *ticketlock) >> { >> return ticketlock->cur_ticket != >> ticketlock->next_ticket; >> } >> -- >> 1.9.1 >> >> >> ______________________________ >> _________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org> >> >> http://lists.linaro.org/ >> mailman/listinfo/lng-odp >> >> >> >> >> -- >> Mike Holmes >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- *Mike Holmes* >> >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >
Looks like a 2.4% improvement + a concern about portability against using bool. Good reasons to document not using it in ODP, although you may need a lot of bool tests to notice anything very measurable. On 9 July 2014 15:19, Maxim Uvarov <maxim.uvarov@linaro.org> wrote: > What is faster bool or int? Just serious :) > > For init function bool is ok, it might go to one byte. But for regular > per-packet/per-buffer checks? What is preferred to cmp, jal and other > branch instruction? > For some reason linux kernel does not have bool for check functions. For > that might be some reason. > > Quick test shows that int is faster: > #include <stdio.h> > #include <stdbool.h> > > int main() > > { > unsigned long long i,j; > > #ifdef TEST_BOOL > bool z=0,x=0,c=0; > #warning "bool version" > #else > int z=0,x=0,c=0; > #warning "int version" > #endif > for (i = 0; i < 10000000000; i++) > z = i & 1; > for (j = 0; i < 1000000000; i++) { > c = j & 1; > if ((c + z) > 0) > x = 1; > else > x = c & 1; > } > > printf("z %d x %d c %d\n", z, x, c); > } > > > maxim@maxim-lap:/tmp/test$ time ./test > z 1 x 0 c 0 > > real 0m32.743s > user 0m32.712s > sys 0m0.000s > maxim@maxim-lap:/tmp/test$ time ./test_int > z 1 x 0 c 0 > > real 0m31.971s > user 0m31.944s > sys 0m0.000s > > BR, > Maxim. > > > On 07/09/2014 11:10 PM, Bill Fischofer wrote: > >> A bit of research shows that the C99 native type is actually _Bool and >> these hold the value 0 and 1. By contrast, the type 'bool' as well as >> 'true' and 'false' are macros defined in <stdbool.h> that map to these. >> See http://pubs.opengroup.org/onlinepubs/009695399/basedefs/ >> stdbool.h.html for details. >> >> Bill >> >> >> On Wed, Jul 9, 2014 at 2:05 PM, Mike Holmes <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> wrote: >> >> I don't think is is restricted to C++ >> >> http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf >> >> I may have the wrong doc but bool is part of C99 if the above is >> correct I think. >> >> >> On 9 July 2014 14:57, Maxim Uvarov <maxim.uvarov@linaro.org >> <mailto:maxim.uvarov@linaro.org>> wrote: >> >> On 07/09/2014 02:28 AM, Victor Kamensky wrote: >> >> On 8 July 2014 10:27, Bill Fischofer >> <bill.fischofer@linaro.org >> <mailto:bill.fischofer@linaro.org>> wrote: >> >> ODP has stated that we require C99. >> >> Sorry, I don't recall that. I remember that C99 compliance >> was discussed >> and I don't have anything against it. C99 compliance does >> not prevent code >> to be C89/C90 compliant. I.e one may state if C99 >> compliant client >> uses ODP library it should work seamlessly. >> >> Have requirement for client of ODP library to be C99 >> compliant is >> quite different >> matter. Note what compliance level implementation of ODP >> library uses inside >> does not matter. I care about requirements to client code >> that would include >> ODP library header files. >> >> We'd like to move towards C11 but for >> many that's a bit too aggressive. C99 is now 15 years >> old so that should be >> mature enough for anyone. >> >> I disagree, there is code much older than that :). >> >> In practice old code may have its own #definition of bool. >> So it will be >> quite messy. Although it may sort of work accidentally. >> Other way is to >> #include <stdbool.h> >> >> >> it's c++ include, not C, right? >> >> >> gcc provides bool define there for older not C99 compliant >> code, but I suspect it could be gcc specific and still >> will mess up >> with "other" bool definitions. >> >> I prefer that client visible header files would not use >> bool type. >> >> Thanks, >> Victor >> >> Bill >> >> >> On Tue, Jul 8, 2014 at 12:08 PM, Mike Holmes >> <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> wrote: >> >> True, but I believe we chose this as our standard, >> although again our >> published arch does does not state that yet. >> >> A quick grep of our mailing list shows accepting >> patches to come into >> compliance with C99 - see "[lng-odp] [PATCHv2 0/4] >> patches towards -std=c99 >> compliance" >> >> Mike >> >> >> On 8 July 2014 12:58, Victor Kamensky >> <victor.kamensky@linaro.org >> <mailto:victor.kamensky@linaro.org>> wrote: >> >> Does not this make C99 absolute requirement for >> ODP library. I wonder whether it is too strict. >> >> C89/90 do not have bool type. >> >> Thanks, >> Victor >> >> >> On 8 July 2014 09:48, Mike Holmes >> <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> wrote: >> >> Signed-off-by: Mike Holmes >> <mike.holmes@linaro.org >> <mailto:mike.holmes@linaro.org>> >> >> --- >> include/odp_buffer.h | 4 ++-- >> include/odp_coremask.h | 11 ++++++++--- >> include/odp_ticketlock.h | 4 ++-- >> platform/linux-generic/odp_buffer.c >> | 2 +- >> platform/linux-generic/odp_coremask.c >> | 2 +- >> platform/linux-generic/odp_ticketlock.c >> | 2 +- >> 6 files changed, 15 insertions(+), 10 >> deletions(-) >> >> diff --git a/include/odp_buffer.h >> b/include/odp_buffer.h >> index d8577fd..c1c9bef 100644 >> --- a/include/odp_buffer.h >> +++ b/include/odp_buffer.h >> @@ -71,9 +71,9 @@ int >> odp_buffer_type(odp_buffer_t buf); >> * >> * @param buf Buffer handle >> * >> - * @return 1 if valid, otherwise 0 >> + * @return true if valid, otherwise false >> */ >> -int odp_buffer_is_valid(odp_buffer_t buf); >> +bool odp_buffer_is_valid(odp_buffer_t buf); >> >> /** >> * Print buffer metadata to STDOUT >> diff --git a/include/odp_coremask.h >> b/include/odp_coremask.h >> index 141cb6a..f5a9a0b 100644 >> --- a/include/odp_coremask.h >> +++ b/include/odp_coremask.h >> @@ -109,9 +109,9 @@ void >> odp_coremask_clr(int core, odp_coremask_t >> *mask); >> * Test if core is a member of mask >> * @param core Core number >> * @param mask Core mask to check if >> core num set or not >> - * @return non-zero if set otherwise 0 >> + * @return true if the core mask is >> set, otherwise false >> */ >> -int odp_coremask_isset(int core, const >> odp_coremask_t *mask); >> +bool odp_coremask_isset(int core, const >> odp_coremask_t *mask); >> >> /** >> * Count number of cores in mask >> @@ -163,8 +163,13 @@ static inline void >> odp_coremask_xor(odp_coremask_t >> *dest, odp_coremask_t *src1, >> >> /** >> * Test if two masks contain the same cores >> + * >> + * @param mask1 First mask to compare >> + * @param mask2 Second mask to compare >> + * >> + * @return true is they are equal or >> false otherwise >> */ >> -static inline int >> odp_coremask_equal(odp_coremask_t *mask1, >> +static inline bool >> odp_coremask_equal(odp_coremask_t *mask1, >> odp_coremask_t *mask2) >> { >> return (mask1->_u64[0] == >> mask2->_u64[0]); >> diff --git a/include/odp_ticketlock.h >> b/include/odp_ticketlock.h >> index 6277a18..2e61ef6 100644 >> --- a/include/odp_ticketlock.h >> +++ b/include/odp_ticketlock.h >> @@ -71,9 +71,9 @@ void >> odp_ticketlock_unlock(odp_ticketlock_t >> *ticketlock); >> * >> * @param ticketlock Ticketlock >> * >> - * @return 1 if the lock is locked, >> otherwise 0. >> + * @return true if the ticket lock is >> locked, otherwise false >> */ >> -int >> odp_ticketlock_is_locked(odp_ticketlock_t >> *ticketlock); >> +bool >> odp_ticketlock_is_locked(odp_ticketlock_t >> *ticketlock); >> >> >> #ifdef __cplusplus >> diff --git >> a/platform/linux-generic/odp_buffer.c >> b/platform/linux-generic/odp_buffer.c >> index 0169eec..41b6a4c 100644 >> --- a/platform/linux-generic/odp_buffer.c >> +++ b/platform/linux-generic/odp_buffer.c >> @@ -36,7 +36,7 @@ int >> odp_buffer_type(odp_buffer_t buf) >> } >> >> >> -int odp_buffer_is_valid(odp_buffer_t buf) >> +bool odp_buffer_is_valid(odp_buffer_t buf) >> { >> odp_buffer_bits_t handle; >> >> diff --git >> a/platform/linux-generic/odp_coremask.c >> b/platform/linux-generic/odp_coremask.c >> index c55eb72..c98a10a 100644 >> --- a/platform/linux-generic/odp_coremask.c >> +++ b/platform/linux-generic/odp_coremask.c >> @@ -81,7 +81,7 @@ void >> odp_coremask_clr(int core, odp_coremask_t >> *mask) >> } >> >> >> -int odp_coremask_isset(int core, const >> odp_coremask_t *mask) >> +bool odp_coremask_isset(int core, const >> odp_coremask_t *mask) >> { >> /* should not be more than 63 >> * core no. should be from 0..63= >> 64bit >> diff --git >> a/platform/linux-generic/odp_ticketlock.c >> b/platform/linux-generic/odp_ticketlock.c >> index be5b885..6cc5285 100644 >> --- a/platform/linux-generic/odp_ticketlock.c >> +++ b/platform/linux-generic/odp_ticketlock.c >> @@ -45,7 +45,7 @@ void >> odp_ticketlock_unlock(odp_ticketlock_t >> *ticketlock) >> } >> >> >> -int >> odp_ticketlock_is_locked(odp_ticketlock_t >> *ticketlock) >> +bool >> odp_ticketlock_is_locked(odp_ticketlock_t >> *ticketlock) >> { >> return ticketlock->cur_ticket != >> ticketlock->next_ticket; >> } >> -- >> 1.9.1 >> >> >> ______________________________ >> _________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org> >> >> http://lists.linaro.org/ >> mailman/listinfo/lng-odp >> >> >> >> >> -- >> Mike Holmes >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org >> <mailto:lng-odp@lists.linaro.org> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >> >> -- *Mike Holmes* >> >> Linaro Technical Manager / Lead >> LNG - ODP >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> http://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >
diff --git a/include/odp_buffer.h b/include/odp_buffer.h index d8577fd..c1c9bef 100644 --- a/include/odp_buffer.h +++ b/include/odp_buffer.h @@ -71,9 +71,9 @@ int odp_buffer_type(odp_buffer_t buf); * * @param buf Buffer handle * - * @return 1 if valid, otherwise 0 + * @return true if valid, otherwise false */ -int odp_buffer_is_valid(odp_buffer_t buf); +bool odp_buffer_is_valid(odp_buffer_t buf); /** * Print buffer metadata to STDOUT diff --git a/include/odp_coremask.h b/include/odp_coremask.h index 141cb6a..f5a9a0b 100644 --- a/include/odp_coremask.h +++ b/include/odp_coremask.h @@ -109,9 +109,9 @@ void odp_coremask_clr(int core, odp_coremask_t *mask); * Test if core is a member of mask * @param core Core number * @param mask Core mask to check if core num set or not - * @return non-zero if set otherwise 0 + * @return true if the core mask is set, otherwise false */ -int odp_coremask_isset(int core, const odp_coremask_t *mask); +bool odp_coremask_isset(int core, const odp_coremask_t *mask); /** * Count number of cores in mask @@ -163,8 +163,13 @@ static inline void odp_coremask_xor(odp_coremask_t *dest, odp_coremask_t *src1, /** * Test if two masks contain the same cores + * + * @param mask1 First mask to compare + * @param mask2 Second mask to compare + * + * @return true is they are equal or false otherwise */ -static inline int odp_coremask_equal(odp_coremask_t *mask1, +static inline bool odp_coremask_equal(odp_coremask_t *mask1, odp_coremask_t *mask2) { return (mask1->_u64[0] == mask2->_u64[0]); diff --git a/include/odp_ticketlock.h b/include/odp_ticketlock.h index 6277a18..2e61ef6 100644 --- a/include/odp_ticketlock.h +++ b/include/odp_ticketlock.h @@ -71,9 +71,9 @@ void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock); * * @param ticketlock Ticketlock * - * @return 1 if the lock is locked, otherwise 0. + * @return true if the ticket lock is locked, otherwise false */ -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock); #ifdef __cplusplus diff --git a/platform/linux-generic/odp_buffer.c b/platform/linux-generic/odp_buffer.c index 0169eec..41b6a4c 100644 --- a/platform/linux-generic/odp_buffer.c +++ b/platform/linux-generic/odp_buffer.c @@ -36,7 +36,7 @@ int odp_buffer_type(odp_buffer_t buf) } -int odp_buffer_is_valid(odp_buffer_t buf) +bool odp_buffer_is_valid(odp_buffer_t buf) { odp_buffer_bits_t handle; diff --git a/platform/linux-generic/odp_coremask.c b/platform/linux-generic/odp_coremask.c index c55eb72..c98a10a 100644 --- a/platform/linux-generic/odp_coremask.c +++ b/platform/linux-generic/odp_coremask.c @@ -81,7 +81,7 @@ void odp_coremask_clr(int core, odp_coremask_t *mask) } -int odp_coremask_isset(int core, const odp_coremask_t *mask) +bool odp_coremask_isset(int core, const odp_coremask_t *mask) { /* should not be more than 63 * core no. should be from 0..63= 64bit diff --git a/platform/linux-generic/odp_ticketlock.c b/platform/linux-generic/odp_ticketlock.c index be5b885..6cc5285 100644 --- a/platform/linux-generic/odp_ticketlock.c +++ b/platform/linux-generic/odp_ticketlock.c @@ -45,7 +45,7 @@ void odp_ticketlock_unlock(odp_ticketlock_t *ticketlock) } -int odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) +bool odp_ticketlock_is_locked(odp_ticketlock_t *ticketlock) { return ticketlock->cur_ticket != ticketlock->next_ticket; }
Signed-off-by: Mike Holmes <mike.holmes@linaro.org> --- include/odp_buffer.h | 4 ++-- include/odp_coremask.h | 11 ++++++++--- include/odp_ticketlock.h | 4 ++-- platform/linux-generic/odp_buffer.c | 2 +- platform/linux-generic/odp_coremask.c | 2 +- platform/linux-generic/odp_ticketlock.c | 2 +- 6 files changed, 15 insertions(+), 10 deletions(-)