Move to using C99 int types and deprecating zephyr int types #25679

galak · 2020-05-27T20:10:47Z

No description provided.

ceolin · 2020-05-28T06:34:12Z

I'm totally in favor of using standard stdint types .

include/sys/util.h

sjg20 · 2020-05-28T16:00:31Z

These are longer and harder to type. Could we do what linux does instead?

u64, u32, u16, u8 for unsigned

galak · 2020-05-28T16:11:47Z

These are longer and harder to type. Could we do what linux does instead?

u64, u32, u16, u8 for unsigned

The issue is that a lot of code integrated w/zephyr utilizes C99 and those types have leaked into Zephyr. If we plan on getting 2 different types, we'd just leave the zephyr types as is.

galak · 2020-06-01T14:57:12Z

The sanitycheck failure is timeout of a test that passes when run by itself.

marc-hb

Move to using PRIu64/PRId64 instead of %llu/%lld since on native_posix_64 the uint64_t/int64_t type is defined in terms of 'long int' and not 'long long int'.

Dropping the keyword "LP64" in the commit message and documentation would add a lot of relevant background information with very few extra characters. Something like: "Unlike Zephyr, native_posix_64 follows the LP64 model that blabla..."

https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models
https://www.google.com/search?q=__LP64__

These are longer and harder to type.

https://twitter.com/study_web_dev/status/764811712394620928

(Not counting all the time spent trying to understand Zephyr types - and I barely did)

Could we do what linux does instead?

The Linux kernel doesn't look like it will ever care about POSIX-looking code.

On 64-bit arch's like AARCH64, x86_64, or riscv-64 the way {u}int64_t will differ from how its defined on 32-bit arch's. Do the same trick we do for {u}int32_t for {u}int64_t so that the type is the same regardless of what we are building for. This keeps things like %lld working the same regardless of 32-bit or 64-bit arch. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

Pull in various module updates for C99 type change Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

git grep -l 'u\(8\|16\|32\|64\)_t' | \ xargs sed -i "s/u\(8\|16\|32\|64\)_t/uint\1_t/g" git grep -l 's\(8\|16\|32\|64\)_t' | \ xargs sed -i "s/s\(8\|16\|32\|64\)_t/int\1_t/g" Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

Move to using PRIu64/PRId64 instead of %llu/%lld since on native_posix_64 the uint64_t/int64_t type is defined in terms of 'long int' and not 'long long int'. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

As the int types defined in include/zephyr/types.h are typdef's we utilize a Kconfig option (LEGACY_ZEPHYR_INT_TYPES) to enable/disable the support for them. By default to LEGACY_ZEPHYR_INT_TYPES not being enabled and add an explicit test to ensure the types continue to function until removed in the future. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

Now that we are standardizing C99 integer types we can revert the commit that warned about C99 type usage. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

andyross

I can't believe we're doing this again. But OK, quick spot checks show that the scripting did the right thing. Please rip the bandaid off and merge this quickly so we can get past the resulting rebase hell.

nashif · 2020-06-06T16:01:59Z

I am not fond of calling old types legacy, legacy means that you want to maintain those for a lengthy period of time where deprecated is more strict and follows deprecation policies.

carlescufi

Please add an entry in the API Changes section of the 2.4 release notes in this PR

carlescufi · 2020-06-06T17:06:09Z

Kconfig.zephyr

+	help
+	  Allows use of the deprecated Zephyr integer typedefs defined in
+	  Zephyr 2.3 and previous versions.
+


@galak could you please mention the types by name here?

carlescufi · 2020-06-06T17:08:31Z

I am not fond of calling old types legacy, legacy means that you want to maintain those for a lengthy period of time where deprecated is more strict and follows deprecation policies.

It's what we do when we deprecate via Kconfig option. At least it's what we've done so far (timeouts and DT, which will both go away in 2 releases), so for consistency I think we should keep the same naming scheme.

It's mentioned here: https://docs.zephyrproject.org/latest/development_process/api_lifecycle.html#deprecated

nashif · 2020-06-06T17:17:08Z

t's what we do when we deprecate via Kconfig option. At least it's what we've done so far (timeouts and DT, which will both go away in 2 releases), so for consistency I think we should keep the same naming scheme.

It's mentioned here: https://docs.zephyrproject.org/latest/development_process/api_lifecycle.html#deprecated

well, just because we have what I think is a mistake, does not mean we have to continue with that mistake, and all of this happened just recently...

https://www.linkedin.com/pulse/deprecated-vs-legacy-ayoub-moustaid/

the way I see it:

deprecated: is something that you should not use and will be removed at some point following the deprecation policy. (2 releases)
legacy: won't be removed for backwards compatibility or other reasons.

Are we going to keep those types more than 2 releases?

carlescufi · 2020-06-06T17:23:57Z

Are we going to keep those types more than 2 releases?

No, I don't think so. So in that case I suggest we change the doc, because right now (mostly because of the timeout and DT Kconfig options using LEGACY even though they'll be removed after 2 releases) it recommends using LEGACY and not DEPRECATED in the Kconfig option name.
I'm not against changing policy, I just want us to be consistent.

pabigot

Is samples/tests: Convert use of %ll{u,d} to PRI{u,d}64 intended to be comprehensive? git grep '%ll' tests/ finds a few more cases.

galak · 2020-06-08T13:05:05Z

Is samples/tests: Convert use of %ll{u,d} to PRI{u,d}64 intended to be comprehensive? git grep '%ll' tests/ finds a few more cases.

Only the samples/tests that get built on native_posix_64 were touched.

galak · 2020-06-08T13:06:09Z

@carlescufi I suggest we merge this and deal with any Kconfig/relnote fixes a follow up PRs.

galak · 2020-06-08T13:09:34Z

An explicit link the shippable run: https://app.shippable.com/github/zephyrproject-rtos/zephyr/runs/74656/summary/console

Add top line note about C99 integer types being the default Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

galak · 2020-06-08T13:32:29Z

@carlescufi & @nashif opened up PR #26047 to continue the legacy/deprecated naming discussion.

This comment has been minimized.

Sign in to view

galak linked an issue May 28, 2020 that may be closed by this pull request

Move to C99 integer types and deprecate zephyr specific types #25327

Closed

aescolar reviewed May 28, 2020

View reviewed changes

include/sys/util.h Outdated Show resolved Hide resolved

galak force-pushed the c99 branch 2 times, most recently from d83049d to 3110f62 Compare May 30, 2020 20:23

galak force-pushed the c99 branch from 3110f62 to 3bc28e7 Compare June 2, 2020 06:12

galak added the TSC Topics that need TSC discussion label Jun 2, 2020

marc-hb reviewed Jun 2, 2020

View reviewed changes

galak force-pushed the c99 branch from 3bc28e7 to 5249579 Compare June 3, 2020 02:21

This was referenced Jun 4, 2020

nrfx_glue: Convert from Zephyr to C99 types zephyrproject-rtos/hal_nordic#42

Merged

usb: Convert from Zephyr to C99 types zephyrproject-rtos/hal_nxp#43

Merged

zephyr: Convert from Zephyr to C99 types zephyrproject-rtos/mcumgr#24

Closed

galak force-pushed the c99 branch 3 times, most recently from e461a14 to 2c3e8bc Compare June 5, 2020 20:31

galak marked this pull request as ready for review June 5, 2020 20:33

galak requested review from albertofloyd, alexanderwachter, anangl, andrewboie, andyross, arnopo, avisconti, bwitherspoon, carlescufi and carlocaione as code owners June 5, 2020 20:33

galak added 6 commits June 6, 2020 07:30

west.yml: Update modules for C99 types

24d2271

Pull in various module updates for C99 type change Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

zephyr: replace zephyr integer types with C99 types

9b4b3c7

git grep -l 'u\(8\|16\|32\|64\)_t' | \ xargs sed -i "s/u\(8\|16\|32\|64\)_t/uint\1_t/g" git grep -l 's\(8\|16\|32\|64\)_t' | \ xargs sed -i "s/s\(8\|16\|32\|64\)_t/int\1_t/g" Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

samples/tests: Convert use of %ll{u,d} to PRI{u,d}64

0163a78

Move to using PRIu64/PRId64 instead of %llu/%lld since on native_posix_64 the uint64_t/int64_t type is defined in terms of 'long int' and not 'long long int'. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

Revert "checkpatch: update checkpatch to warn about C99 type usage"

1d75609

Now that we are standardizing C99 integer types we can revert the commit that warned about C99 type usage. Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

galak force-pushed the c99 branch from 820bb21 to db653aa Compare June 6, 2020 12:30

henrikbrixandersen approved these changes Jun 6, 2020

View reviewed changes

andyross approved these changes Jun 6, 2020

View reviewed changes

carlescufi requested changes Jun 6, 2020

View reviewed changes

jukkar approved these changes Jun 6, 2020

View reviewed changes

ruuddw approved these changes Jun 8, 2020

View reviewed changes

pabigot reviewed Jun 8, 2020

View reviewed changes

doc: relnotes: Add note about C99 types.

b8ab557

Add top line note about C99 integer types being the default Signed-off-by: Kumar Gala <kumar.gala@linaro.org>

galak force-pushed the c99 branch from db653aa to b8ab557 Compare June 8, 2020 13:11

galak requested a review from carlescufi June 8, 2020 13:12

carlescufi approved these changes Jun 8, 2020

View reviewed changes

galak merged commit a2d4292 into zephyrproject-rtos:master Jun 8, 2020

galak deleted the c99 branch June 8, 2020 13:26

rlubos mentioned this pull request Jun 8, 2020

net: openthread: add pskd join loop #25589

Closed

MaureenHelm mentioned this pull request Jun 12, 2020

soc: Add support for LPC11U6X soc series #25837

Merged

mnkp mentioned this pull request Jun 13, 2020

stdint.h: make redefinition of stdint types optional #19912

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move to using C99 int types and deprecating zephyr int types #25679

Move to using C99 int types and deprecating zephyr int types #25679

galak commented May 27, 2020

This comment has been minimized.

ceolin commented May 28, 2020

sjg20 commented May 28, 2020

galak commented May 28, 2020

galak commented Jun 1, 2020

marc-hb left a comment

andyross left a comment

nashif commented Jun 6, 2020

carlescufi left a comment

carlescufi Jun 6, 2020

carlescufi commented Jun 6, 2020

nashif commented Jun 6, 2020

carlescufi commented Jun 6, 2020

pabigot left a comment

galak commented Jun 8, 2020

galak commented Jun 8, 2020

galak commented Jun 8, 2020

galak commented Jun 8, 2020

Move to using C99 int types and deprecating zephyr int types #25679

Move to using C99 int types and deprecating zephyr int types #25679

Conversation

galak commented May 27, 2020

This comment has been minimized.

ceolin commented May 28, 2020

sjg20 commented May 28, 2020

galak commented May 28, 2020

galak commented Jun 1, 2020

marc-hb left a comment

Choose a reason for hiding this comment

andyross left a comment

Choose a reason for hiding this comment

nashif commented Jun 6, 2020

carlescufi left a comment

Choose a reason for hiding this comment

carlescufi Jun 6, 2020

Choose a reason for hiding this comment

carlescufi commented Jun 6, 2020

nashif commented Jun 6, 2020

carlescufi commented Jun 6, 2020

pabigot left a comment

Choose a reason for hiding this comment

galak commented Jun 8, 2020

galak commented Jun 8, 2020

galak commented Jun 8, 2020

galak commented Jun 8, 2020