Change in ...coreboot[master]: Get rid of PRIu64

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33823

Change subject: Get rid of PRIu64
......................................................................

Get rid of PRIu64

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/lib/timestamp.c
M util/cbfstool/rmodule.c
6 files changed, 2 insertions(+), 16 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/1

diff --git a/src/arch/arm/include/stdint.h b/src/arch/arm/include/stdint.h
index ede0ec9..ae5ff22 100644
--- a/src/arch/arm/include/stdint.h
+++ b/src/arch/arm/include/stdint.h
@@ -101,9 +101,6 @@
 #ifndef UINT64_C
 #define UINT64_C(c) c ## ULL
 #endif
-#ifndef PRIu64
-#define PRIu64 "llu"
-#endif

#undef __HAVE_LONG_LONG__

diff --git a/src/arch/arm64/include/stdint.h b/src/arch/arm64/include/stdint.h
index cb07a07..e40a9f5 100644
--- a/src/arch/arm64/include/stdint.h
+++ b/src/arch/arm64/include/stdint.h
@@ -79,8 +79,5 @@
 #ifndef UINT64_MAX
 # define UINT64_MAX (18446744073709551615ULL)
 #endif
-#ifndef PRIu64
-#define PRIu64 "llu"
-#endif

#endif /* ARM64_STDINT_H */
diff --git a/src/arch/riscv/include/stdint.h b/src/arch/riscv/include/stdint.h
index 76f0d1b..eaaba18 100644
--- a/src/arch/riscv/include/stdint.h
+++ b/src/arch/riscv/include/stdint.h
@@ -73,7 +73,4 @@
 typedef long intptr_t;
 typedef unsigned long uintptr_t;

-/* FIXME: This is used in some print code and may be removed in the future. */
-#define PRIu64  "llu"
-
 #endif /* RISCV_STDINT_H */
diff --git a/src/arch/x86/include/stdint.h b/src/arch/x86/include/stdint.h
index 6c40002..262c6a3 100644
--- a/src/arch/x86/include/stdint.h
+++ b/src/arch/x86/include/stdint.h
@@ -102,11 +102,6 @@
 #ifndef UINT64_C
 #define UINT64_C(c) c ## ULL
 #endif
-#ifndef PRIu64
-#define PRIu64 "llu"
-
-#endif
-

#undef __HAVE_LONG_LONG__

diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c
index adacf6b..347e937 100644
--- a/src/lib/timestamp.c
+++ b/src/lib/timestamp.c
@@ -180,7 +180,7 @@
    tse->entry_stamp = ts_time - ts_table->base_time;

if (CONFIG(TIMESTAMPS_ON_CONSOLE))
-		printk(BIOS_SPEW, "Timestamp - %s: %" PRIu64 "\n",
+		printk(BIOS_SPEW, "Timestamp - %s: %llu\n",
    			timestamp_name(id), ts_time);

if (ts_table->num_entries == ts_table->max_entries)
diff --git a/util/cbfstool/rmodule.c b/util/cbfstool/rmodule.c
index 80e8911..c1fcd13 100644
--- a/util/cbfstool/rmodule.c
+++ b/util/cbfstool/rmodule.c
@@ -328,7 +328,7 @@
    	return -1;

nrelocs = ctx->nrelocs;
-	INFO("%" PRIu64 " relocations to be emitted.\n", nrelocs);
+	INFO("%llu relocations to be emitted.\n", nrelocs);
    if (!nrelocs)
    	return 0;

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#2).

Change subject: Get rid of PRIu64
......................................................................

Get rid of PRIu64

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/lib/timestamp.c
5 files changed, 1 insertion(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/2

Change in ...coreboot[master]: src: Get rid of PRIu64

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#3).

Change subject: src: Get rid of PRIu64
......................................................................

src: Get rid of PRIu64

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/lib/timestamp.c
5 files changed, 1 insertion(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/3

Change in ...coreboot[master]: src: Get rid of PRIu64

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Get rid of PRIu64
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/33823/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33823/3//COMMIT_MSG@8 
PS3, Line 8: 
Why?

Change in ...coreboot[master]: src: Get rid of PRIu64

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#4).

Change subject: src: Get rid of PRIu64
......................................................................

src: Get rid of PRIu64

PRIu64 is not used anymore in printk().

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/lib/timestamp.c
5 files changed, 1 insertion(+), 15 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/4

Change in ...coreboot[master]: src: Get rid of PRIu64

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Get rid of PRIu64
......................................................................

Patch Set 4:

I would rather that we get a proper <inttypes.h> with all of these, to be honest. vboot uses these sometimes (I think not in the part linked into coreboot atm, but that may change), and it's just a standard C library feature that doesn't really cost anything to support and is nice to have available. Maybe we can just copy the inttypes.h from libpayload? (Although I think a few of the definitions there look sketchy and should maybe be double-checked to make sure they do the right thing for all archs we support these days.)

Change in ...coreboot[master]: src: Get rid of PRIu64

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Get rid of PRIu64
......................................................................

Patch Set 4: Code-Review-1

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#5).

Change subject: src: Use standard <inttypes.h>
......................................................................

src: Use standard <inttypes.h>

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/include/inttypes.h
M src/lib/timestamp.c
6 files changed, 262 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/5

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 5:

(15 comments)

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@251 
PS5, Line 251: __BEGIN_DECLS
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@253 
PS5, Line 253: imaxdiv_t	imaxdiv(intmax_t, intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@253 
PS5, Line 253: imaxdiv_t	imaxdiv(intmax_t, intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@254 
PS5, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'const char *' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@254 
PS5, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'char **' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@254 
PS5, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@255 
PS5, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'const char *' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@255 
PS5, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'char **' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@255 
PS5, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@256 
PS5, Line 256: intmax_t	wcstoimax(const __wchar_t * __restrict,
"foo * bar" should be "foo *bar"

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@256 
PS5, Line 256: intmax_t	wcstoimax(const __wchar_t * __restrict,
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@257 
PS5, Line 257: 		    __wchar_t ** __restrict, int);
"foo ** bar" should be "foo **bar"

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@258 
PS5, Line 258: uintmax_t	wcstoumax(const __wchar_t * __restrict,
"foo * bar" should be "foo *bar"

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@258 
PS5, Line 258: uintmax_t	wcstoumax(const __wchar_t * __restrict,
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/5/src/include/inttypes.h@259 
PS5, Line 259: 		    __wchar_t ** __restrict, int);
"foo ** bar" should be "foo **bar"

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#6).

Change subject: src: Use standard <inttypes.h>
......................................................................

src: Use standard <inttypes.h>

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/include/inttypes.h
M src/lib/timestamp.c
6 files changed, 264 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/6

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 6:

(14 comments)

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@251 
PS6, Line 251: __BEGIN_DECLS
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@253 
PS6, Line 253: imaxdiv_t	imaxdiv(intmax_t, intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@253 
PS6, Line 253: imaxdiv_t	imaxdiv(intmax_t, intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@254 
PS6, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'const char *' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@254 
PS6, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'char **' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@254 
PS6, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@255 
PS6, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'const char *' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@255 
PS6, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'char **' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@255 
PS6, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@257 
PS6, Line 257: intmax_t	wcstoimax(const __wchar_t * __restrict,
"foo * bar" should be "foo *bar"

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@258 
PS6, Line 258: 		    __wchar_t ** __restrict, int);
"foo ** bar" should be "foo **bar"

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@259 
PS6, Line 259: uintmax_t	wcstoumax(const __wchar_t * __restrict,
"foo * bar" should be "foo *bar"

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@259 
PS6, Line 259: uintmax_t	wcstoumax(const __wchar_t * __restrict,
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/6/src/include/inttypes.h@260 
PS6, Line 260: 		    __wchar_t ** __restrict, int);
"foo ** bar" should be "foo **bar"

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

Hello Julius Werner, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#7).

Change subject: src: Use standard <inttypes.h>
......................................................................

src: Use standard <inttypes.h>

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/include/inttypes.h
M src/lib/timestamp.c
6 files changed, 264 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/7

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7:

(14 comments)

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@252 
PS7, Line 252: intmax_t	imaxabs(intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@253 
PS7, Line 253: imaxdiv_t	imaxdiv(intmax_t, intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@253 
PS7, Line 253: imaxdiv_t	imaxdiv(intmax_t, intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@254 
PS7, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'const char *' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@254 
PS7, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'char **' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@254 
PS7, Line 254: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@255 
PS7, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'const char *' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@255 
PS7, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'char **' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@255 
PS7, Line 255: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@257 
PS7, Line 257: intmax_t	wcstoimax(const __wchar_t * __restrict,
"foo * bar" should be "foo *bar"

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@258 
PS7, Line 258: 		    __wchar_t ** __restrict, int);
"foo ** bar" should be "foo **bar"

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@259 
PS7, Line 259: uintmax_t	wcstoumax(const __wchar_t * __restrict,
"foo * bar" should be "foo *bar"

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@259 
PS7, Line 259: uintmax_t	wcstoumax(const __wchar_t * __restrict,
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@260 
PS7, Line 260: 		    __wchar_t ** __restrict, int);
"foo ** bar" should be "foo **bar"

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7: Code-Review+2

works for now (bonus points for using openbsd's header). if we ever run into weird ABIs that have different ideas of how to do this, we can revisit things

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7:

I generally like the idea to keep our headers as close
to the standard as possible.

Regarding this change:

* inttypes.h like the stdint.h counterpart is arch specific. e.g. if `int64_t` is defined as `long long int` then u need `ll` prefix, if it is defined as `long int` you only need `l`.
* Adding all the standard types even if we don't use all of them increases the maintenance burden for naught. e.g. why add *int_fast*/*int_least* if nobody is ever going to use them?

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7:
> Patch Set 7:
> Adding all the standard types even if we don't use all of them increases the maintenance burden for naught. e.g. why add *int_fast*/*int_least* if nobody is ever going to use them?

It's taken verbatim from a project that maintains it, including a version string even for simple updates (yay CVS ;-) ), so I'm not seeing much of a maintenance issue unless we need to diverge from its upstream.

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7:
> > Adding all the standard types even if we don't use all of them increases the maintenance burden for naught. e.g. why add *int_fast*/*int_least* if nobody is ever going to use them?
>
> It's taken verbatim from a project that maintains it, including a version string even for simple updates (yay CVS ;-) ), so I'm not seeing much of a maintenance issue unless we need to diverge from its upstream.

I was more thinking about what this may become if we can't use an
exact copy. But it seems we can, with the following assumptions:

* Only the size of pointers and (unsigned) long int differs between our supported data models, and
* pointers have the same size as long int.

But, if we make these assumptions, then we also don't need stdint.h
to be arch specific :)

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7: Code-Review-1

I believe this deserves more discussion. I've given a negative score just to prevent accidental submissions.

Consider this review score a -0.5, rounded down to the nearest non-zero integer :)

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7:
> I was more thinking about what this may become if we can't use an
> exact copy. But it seems we can, with the following assumptions:
> Only the size of pointers and (unsigned) long int differs between our supported data models, and
> pointers have the same size as long int.
>
> But, if we make these assumptions, then we also don't need stdint.h
> to be arch specific :)

The idea came up that if the above assumptions are true
for all our arch's, we could have a single `stdint.h`.
Thoughts?

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/#/c/33823/7//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/33823/7//COMMIT_MSG@8 
PS7, Line 8: 
Please elaborate.

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7: Code-Review+1

(3 comments)
> The idea came up that if the above assumptions are true
> for all our arch's, we could have a single `stdint.h`.
> Thoughts?

I we can make that work across all platforms (sounds like we can?) I think it's a good idea.

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@60 
PS7, Line 60: j
"j" is not supported by our vtxprintf(). Either we have to add it there, or replace all "j" in here with "ll".

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@154 
PS7, Line 154: /* fscanf macros for signed integers */
There's no fscanf() in coreboot, so we don't really need any of the SCN (although they don't hurt either).

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@246 
PS7, Line 246: typedef struct {
I don't think we need anything below here, so should remove it.

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@246 
PS7, Line 246: typedef struct {
> I don't think we need anything below here, so should remove it.

t's taken  from CVS. Do we need to diverge from its upstream?

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 7:

(1 comment)

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/#/c/33823/7/src/include/inttypes.h@246 
PS7, Line 246: typedef struct {
> t's taken  from CVS. […]

I mean, just cutting out one contiguous block of lines is a pretty easy divergence to manage. I think we shouldn't define prototypes for functions that we don't implement, it just creates confusion.

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

Hello Kyösti Mälkki, ron minnich, Julius Werner, Angel Pons, Arthur Heymans, Patrick Rudolph, build bot (Jenkins), Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#8).

Change subject: src: Use standard <inttypes.h>
......................................................................

src: Use standard <inttypes.h>

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/include/inttypes.h
M src/lib/timestamp.c
6 files changed, 264 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/8

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 8:

(15 comments)

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@253 
PS8, Line 253: intmax_t	imaxabs(intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@254 
PS8, Line 254: imaxdiv_t	imaxdiv(intmax_t, intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@254 
PS8, Line 254: imaxdiv_t	imaxdiv(intmax_t, intmax_t);
function definition argument 'intmax_t' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@255 
PS8, Line 255: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'const char *' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@255 
PS8, Line 255: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'char **' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@255 
PS8, Line 255: intmax_t	strtoimax(const char *, char **, int);
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@256 
PS8, Line 256: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'const char *' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@256 
PS8, Line 256: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'char **' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@256 
PS8, Line 256: uintmax_t	strtoumax(const char *, char **, int);
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@257 
PS8, Line 257: intmax_t	wcstoimax(const __wchar_t * __restrict,
"foo * bar" should be "foo *bar"

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@257 
PS8, Line 257: intmax_t	wcstoimax(const __wchar_t * __restrict,
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@258 
PS8, Line 258: 		    __wchar_t ** __restrict, int);
"foo ** bar" should be "foo **bar"

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@259 
PS8, Line 259: uintmax_t	wcstoumax(const __wchar_t * __restrict,
"foo * bar" should be "foo *bar"

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@259 
PS8, Line 259: uintmax_t	wcstoumax(const __wchar_t * __restrict,
function definition argument 'int' should also have an identifier name

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@260 
PS8, Line 260: 		    __wchar_t ** __restrict, int);
"foo ** bar" should be "foo **bar"

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 8: Code-Review-1

(1 comment)

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/#/c/33823/8/src/include/inttypes.h@60 
PS8, Line 60: "lld"
this is not a CVS version

Change in ...coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has removed a vote on this change.

Change subject: src: Use standard <inttypes.h>
......................................................................

Removed Code-Review-1 by HAOUAS Elyes

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 8:

I think this is very unlikely to ever need a change. So
it shouldn't matter if we use a direct copy from some-
where else.

I would prefer to just copy/adapt (s/j/ll/) the useful
parts.

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 8:
> I would prefer to just copy/adapt (s/j/ll/) the useful
> parts.

That would be the PRI* macros, if need be also the SCN*
ones, but definitely no abandoned `#if 0` block full of
clang-format training excercises.

Change in coreboot[master]: src: Use standard <inttypes.h>

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 8:
> > I would prefer to just copy/adapt (s/j/ll/) the useful
> > parts.

Implementing j in vtxprintf() is very easy (CB:34027), so we can just copy-paste those parts.
> That would be the PRI* macros, if need be also the SCN*
> ones, but definitely no abandoned `#if 0` block full of
> clang-format training excercises.

Agreed.

Change in coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

Hello Kyösti Mälkki, ron minnich, Julius Werner, Angel Pons, Jacob Garber, Arthur Heymans, Patrick Rudolph, build bot (Jenkins), Patrick Georgi,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#9).

Change subject: src: Use standard <inttypes.h>
......................................................................

src: Use standard <inttypes.h>

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/include/inttypes.h
M src/lib/timestamp.c
6 files changed, 246 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/9

Change in coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 9:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/7/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/7/src/include/inttypes.h@60 
PS7, Line 60: j
> "j" is not supported by our vtxprintf(). […]

please see https://review.coreboot.org/c/coreboot/+/34027

Change in coreboot[master]: src: Use standard <inttypes.h>

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 10: Code-Review+1

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 10: Code-Review-1

(3 comments)

I think it's actually dangerous to implement these things
for types that (currently) nobody cares about. But if we
really want to cover all these types, I'm ok with it. I
just fear this will have to go through some more rounds.

We should also have a common `stdint.h` ready so it's
obvious that we expect a specific implementation for all
supported architectures. Or maybe even discuss that one
first. For instance, the spotted issue may also mean that
we might want to adapt `stdint`?

https://review.coreboot.org/c/coreboot/+/33823/10/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/10/src/include/inttypes.h@202 
PS10, Line 202: #define	SCNoFAST8		"o"		/* uint_fast8_t */
nope, our current `fast` implementation seems to match the regular one

https://review.coreboot.org/c/coreboot/+/33823/10/src/include/inttypes.h@220 
PS10, Line 220: #define	SCNuFAST8		"u"		/* uint_fast8_t */
nope

https://review.coreboot.org/c/coreboot/+/33823/10/src/include/inttypes.h@238 
PS10, Line 238: #define	SCNxFAST8		"x"		/* uint_fast8_t */
nope

Change in coreboot[master]: src: Use standard <inttypes.h>

[Jacob Garber (Code Review)]

Jacob Garber has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Removed Code-Review+1 by Jacob Garber

Change in coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 10:
> Patch Set 10: Code-Review-1
>
> (3 comments)
>
> I think it's actually dangerous to implement these things
> for types that (currently) nobody cares about. But if we
> really want to cover all these types, I'm ok with it. I
> just fear this will have to go through some more rounds.
>
> We should also have a common `stdint.h` ready so it's
> obvious that we expect a specific implementation for all
> supported architectures. Or maybe even discuss that one
> first. For instance, the spotted issue may also mean that
> we might want to adapt `stdint`?

I agree, we need a common `stdint.h`

Change in coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 10:

(3 comments)

https://review.coreboot.org/c/coreboot/+/33823/10/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/10/src/include/inttypes.h@202 
PS10, Line 202: #define	SCNoFAST8		"o"		/* uint_fast8_t */
> nope, our current `fast` implementation seems to match the regular one

Done

https://review.coreboot.org/c/coreboot/+/33823/10/src/include/inttypes.h@220 
PS10, Line 220: #define	SCNuFAST8		"u"		/* uint_fast8_t */
> nope

Done

https://review.coreboot.org/c/coreboot/+/33823/10/src/include/inttypes.h@238 
PS10, Line 238: #define	SCNxFAST8		"x"		/* uint_fast8_t */
> nope

Done

Change in coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

Hello Kyösti Mälkki, Julius Werner, Angel Pons, Jacob Garber, Arthur Heymans, Patrick Rudolph, build bot (Jenkins), Philipp Hug, Patrick Georgi, ron minnich, Jonathan Neuschäfer, Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#11).

Change subject: src: Use standard <inttypes.h>
......................................................................

src: Use standard <inttypes.h>

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/arch/arm/include/stdint.h
M src/arch/arm64/include/stdint.h
M src/arch/riscv/include/stdint.h
M src/arch/x86/include/stdint.h
M src/include/inttypes.h
M src/lib/timestamp.c
6 files changed, 244 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/11

Change in coreboot[master]: src: Use standard <inttypes.h>

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h@71 
PS11, Line 71: #define	PRIiFAST8		"i"		/* int_fast8_t */
Same problem here that Nico pointed out for the scanning ones. Note that this doesn't actually matter because with the way varargs work on all our architectures (and the way it's implemented in vtxprintf()), it's effectively treated the same anyway (assuming you pass an argument of the right type).

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h@71 
PS11, Line 71: #define	PRIiFAST8		"i"		/* int_fast8_t */
> Same problem here that Nico pointed out for the scanning ones. […]

You cannot pass anything smaller than an `int`, as it would
be promoted. So this seems correct here. See also `PRIi8`.
It's not specific to our implementation but general C semantics,
AIUI.

The scanf conversions are different because we pass pointers
there (and promoting pointer target types would be insane).

Change in coreboot[master]: src: Use standard <inttypes.h>

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h@71 
PS11, Line 71: #define	PRIiFAST8		"i"		/* int_fast8_t */
> You cannot pass anything smaller than an `int`, as it would […]

Yes, that's what I mean by "it doesn't actually matter as long as you pass an argument of the right type". The only time it matters is when you do something like printk(BIOS_DEBUG, "%hhi", 1000), but that's of course not valid anyway. Still, I'd say "hhi" would be more correct for PRIi8 than "i". If we set PRIi8 to "i" we should ask ourselves why we have the code handling h/hh in vtxprintf() anyway, because then it would never get used (maybe the right answer is that we should get rid of it or at least silently ignore those letters instead).

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h@71 
PS11, Line 71: #define	PRIiFAST8		"i"		/* int_fast8_t */
> Yes, that's what I mean by "it doesn't actually matter as long as you pass an argument of the right  […]

It took me some time to understand the vtxprintf() implementation. It looks
odd, but only because it _is_ standard compliant. I assume, because of the
integer promotion C designers had to come up with something for the `h` and
`hh` modifiers that is very unintuitive, "its value shall be converted to
signed char or unsigned char before printing". This has two implications

1. `h` and `hh` modifiers for printing don't relate to the type of the
argument but only restrict the range of the printed value, and
2. `printf("%hhi", 1000)` is valid and that it would overflow is the
only case where an `hh` modifier makes sense for printing.

I guess there are two ways to go from here:

1. Keep it standard compliant, and "i" for the PRIi*{8|16} macros.
2. Ditch `h` and `hh` completely for printing (who's using that?).

It's already implemented, so I would prefer the former.

Change in coreboot[master]: src: Use standard <inttypes.h>

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 11: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h@71 
PS11, Line 71: #define	PRIiFAST8		"i"		/* int_fast8_t */
> It took me some time to understand the vtxprintf() implementation. It looks […]

I don't think the standard requires "i" for PRIi16? At least I'm not seeing it. But I guess if all major C libraries do it that way, it makes sense that we do the same.

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 11:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h@71 
PS11, Line 71: #define	PRIiFAST8		"i"		/* int_fast8_t */
> I don't think the standard requires "i" for PRIi16? At least I'm not seeing it. […]

I don't see a requirement. Simply, `h` for PRIi16 and `hh` for PRIi8 would
be no-ops by definition. If we'd put them here, it would be less confusing
for those that don't know about the integer promotions and more confusing
for those that do know. Nobody can win, I guess.

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 11:

Elyes, please rebase this on CB:34075.

Change in coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

Hello Kyösti Mälkki, Julius Werner, Angel Pons, Jacob Garber, Arthur Heymans, Patrick Rudolph, build bot (Jenkins), Philipp Hug, Patrick Georgi, ron minnich, Jonathan Neuschäfer, Nico Huber,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/33823

to look at the new patch set (#12).

Change subject: src: Use standard <inttypes.h>
......................................................................

src: Use standard <inttypes.h>

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Elyes HAOUAS 
---
M src/include/inttypes.h
M src/lib/timestamp.c
2 files changed, 244 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/12

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 12:

(1 comment)
> Elyes, please rebase this on CB:34075.

There are some changes in CB:34075 to reflect here:

* `least` and `fast` integer types are gone.
* Two redundant PRI* definitions in `stdint.h` should be removed there.
* Something I forgot about, hmmm... maybe that was all.

https://review.coreboot.org/c/coreboot/+/33823/12/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/12/src/include/inttypes.h@22 
PS12, Line 22: #ifdef __cplusplus
I guess we won't need this.

Change in coreboot[master]: src: Use standard <inttypes.h>

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 13:

I know this change has been debated a lot, but I'm not sure we need to introduce this header anymore (or at least not right now). coreboot doesn't use any of these typedefs, and I don't see that changing now that the integer types in stdint.h have been standardized. If or when 3rdparty code like vboot needs to rely on this we could just implement the basic typedefs that are actually needed then and ignore all the weird ones.

Change in coreboot[master]: src: Use standard <inttypes.h>

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 13:
> If or when 3rdparty code like vboot needs to rely on this we could just implement the basic typedefs that are actually needed then and ignore all the weird ones.

...so, can we do that instead then? I don't care how it's done in the details, but I think we should definitely be providing the common definitions like PRIu64 or PRIx32 somehow.

Change in coreboot[master]: src: Use standard <inttypes.h>

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 13:
> Patch Set 13:
> > If or when 3rdparty code like vboot needs to rely on this we could just implement the basic typedefs that are actually needed then and ignore all the weird ones.
>
> ...so, can we do that instead then? I don't care how it's done in the details, but I think we should definitely be providing the common definitions like PRIu64 or PRIx32 somehow.

Alright. Hey Elyes, mind if I start working on this?

Change in coreboot[master]: src: Use standard <inttypes.h>

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 13:
> > If or when 3rdparty code like vboot needs to rely on this we could just implement the basic typedefs that are actually needed then and ignore all the weird ones.
>
> ...so, can we do that instead then? I don't care how it's done in the details, but I think we should definitely be providing the common definitions like PRIu64 or PRIx32 somehow.

I would prefer to have a (reduced) `inttypes.h`. No that often, but sometimes
I miss it. And it would feel weird to look up which file to include instead.

Change in coreboot[master]: src: Use standard <inttypes.h>

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: src: Use standard <inttypes.h>
......................................................................

Patch Set 13:
> Patch Set 13:
> > Patch Set 13:
> > > If or when 3rdparty code like vboot needs to rely on this we could just implement the basic typedefs that are actually needed then and ignore all the weird ones.
> >
> > ...so, can we do that instead then? I don't care how it's done in the details, but I think we should definitely be providing the common definitions like PRIu64 or PRIx32 somehow.
>
> Alright. Hey Elyes, mind if I start working on this?

Please feel free :)

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Jacob Garber (Code Review)]

Jacob Garber has uploaded a new patch set (#14) to the change originally created by HAOUAS Elyes. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

include, lib: Add <inttypes.h> printf macros

In general, third party code (such as vboot) doesn't know what the
underlying types are for the integers in <stdint.h>, so these macros are
useful for portably printing them. Of these definitions, coreboot so far has
only used PRIu64 (in one place), which isn't needed anymore since we know
what the underlying type of a u64 is.

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Jacob Garber 
---
M src/include/inttypes.h
M src/include/stdint.h
M src/lib/timestamp.c
3 files changed, 64 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/14

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

Patch Set 14:

(7 comments)

https://review.coreboot.org/c/coreboot/+/33823/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/33823/3//COMMIT_MSG@8 
PS3, Line 8:
> Why?

Done

https://review.coreboot.org/c/coreboot/+/33823/7//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/33823/7//COMMIT_MSG@8 
PS7, Line 8:
> Please elaborate.

Done

https://review.coreboot.org/c/coreboot/+/33823/12/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/12/src/include/inttypes.h@22 
PS12, Line 22: #ifdef __cplusplus
> I guess we won't need this.

Done

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/11/src/include/inttypes.h@71 
PS11, Line 71: #define	PRIiFAST8		"i"		/* int_fast8_t */
> I don't see a requirement. Simply, `h` for PRIi16 and `hh` for PRIi8 would […]

Done

https://review.coreboot.org/c/coreboot/+/33823/8/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/8/src/include/inttypes.h@60 
PS8, Line 60: "lld"
> this is not a CVS version

Done

https://review.coreboot.org/c/coreboot/+/33823/7/src/include/inttypes.h 
File src/include/inttypes.h:

https://review.coreboot.org/c/coreboot/+/33823/7/src/include/inttypes.h@154 
PS7, Line 154: /* fscanf macros for signed integers */
> There's no fscanf() in coreboot, so we don't really need any of the SCN (although they don't hurt ei […]

Done

https://review.coreboot.org/c/coreboot/+/33823/7/src/include/inttypes.h@246 
PS7, Line 246: typedef struct {
> I mean, just cutting out one contiguous block of lines is a pretty easy divergence to manage. […]

Done

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

Patch Set 14: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/14/src/lib/timestamp.c 
File src/lib/timestamp.c:

https://review.coreboot.org/c/coreboot/+/33823/14/src/lib/timestamp.c@184 
PS14, Line 184: 				timestamp_name(id), ts_time);
Why the change? Anyway, does it fit one line now?

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

Patch Set 14:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/14/src/lib/timestamp.c 
File src/lib/timestamp.c:

https://review.coreboot.org/c/coreboot/+/33823/14/src/lib/timestamp.c@184 
PS14, Line 184: 				timestamp_name(id), ts_time);
> Why the change? Anyway, does it fit one line now?

Mostly to be consistent with the rest of the code base, which doesn't use PRIu64 anywhere else (or any of the other PRI* macros for that matter). I suppose we could start using these macros more widely now, but IMHO they are more verbose and not really necessary since our fixed-width integers don't change.

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

Patch Set 14:

(2 comments)

https://review.coreboot.org/c/coreboot/+/33823/14//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/33823/14//COMMIT_MSG@11 
PS14, Line 11: useful for portably printing them. Of these definitions, coreboot so far has
             : only used PRIu64 (in one place), which isn't needed anymore since we know
overlong lines btw.

https://review.coreboot.org/c/coreboot/+/33823/14/src/lib/timestamp.c 
File src/lib/timestamp.c:

https://review.coreboot.org/c/coreboot/+/33823/14/src/lib/timestamp.c@184 
PS14, Line 184: 				timestamp_name(id), ts_time);
> Mostly to be consistent with the rest of the code base, which doesn't use PRIu64 anywhere else (or a […]

Well, I would see PRIu64 as a courtesy to the reviewer, so they don't
have to know our uint64_t implementation details. Consistency is a good
argument, too. Feel free to remove the line break, though.

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Jacob Garber (Code Review)]

Jacob Garber has uploaded a new patch set (#15) to the change originally created by HAOUAS Elyes. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

include, lib: Add <inttypes.h> printf macros

In general, third party code (such as vboot) doesn't know what the
underlying types are for the integers in <stdint.h>, so these macros are
useful for portably printing them. Of these definitions, coreboot so far
has only used PRIu64 (in one place), which isn't needed anymore since we
know what the underlying type of a u64 is.

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Jacob Garber 
---
M src/include/inttypes.h
M src/include/stdint.h
M src/lib/timestamp.c
3 files changed, 64 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/33823/15

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Jacob Garber (Code Review)]

Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

Patch Set 15:

(1 comment)

https://review.coreboot.org/c/coreboot/+/33823/14//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/33823/14//COMMIT_MSG@11 
PS14, Line 11: useful for portably printing them. Of these definitions, coreboot so far has
             : only used PRIu64 (in one place), which isn't needed anymore since we know
> overlong lines btw.

Oops, thanks.

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

Patch Set 15: Code-Review+2

Change in coreboot[master]: include, lib: Add <inttypes.h> printf macros

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33823 )

Change subject: include, lib: Add <inttypes.h> printf macros
......................................................................

include, lib: Add <inttypes.h> printf macros

In general, third party code (such as vboot) doesn't know what the
underlying types are for the integers in <stdint.h>, so these macros are
useful for portably printing them. Of these definitions, coreboot so far
has only used PRIu64 (in one place), which isn't needed anymore since we
know what the underlying type of a u64 is.

Change-Id: I9e3a300f9b1c38e4831b030ff8af3fed2fa60f14
Signed-off-by: Jacob Garber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33823
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
M src/include/inttypes.h
M src/include/stdint.h
M src/lib/timestamp.c
3 files changed, 64 insertions(+), 8 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/src/include/inttypes.h b/src/include/inttypes.h
index 77c39c4..4e2476d 100644
--- a/src/include/inttypes.h
+++ b/src/include/inttypes.h
@@ -1,4 +1,67 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
 #ifndef INTTYPES_H
 #define INTTYPES_H
+
 #include <stdint.h>
+
+/* int8_t and uint8_t */
+#define PRId8   "d"
+#define PRIi8   "i"
+#define PRIu8   "u"
+#define PRIo8   "o"
+#define PRIx8   "x"
+#define PRIX8   "X"
+
+/* int16_t and uint16_t */
+#define PRId16  "d"
+#define PRIi16  "i"
+#define PRIu16  "u"
+#define PRIo16  "o"
+#define PRIx16  "x"
+#define PRIX16  "X"
+
+/* int32_t and uint32_t */
+#define PRId32  "d"
+#define PRIi32  "i"
+#define PRIu32  "u"
+#define PRIo32  "o"
+#define PRIx32  "x"
+#define PRIX32  "X"
+
+/* int64_t and uint64_t */
+#define PRId64  "lld"
+#define PRIi64  "lli"
+#define PRIu64  "llu"
+#define PRIo64  "llo"
+#define PRIx64  "llx"
+#define PRIX64  "llX"
+
+/* intptr_t and uintptr_t */
+#define PRIdPTR "ld"
+#define PRIiPTR "li"
+#define PRIuPTR "lu"
+#define PRIoPTR "lo"
+#define PRIxPTR "lx"
+#define PRIXPTR "lX"
+
+/* intmax_t and uintmax_t */
+#define PRIdMAX "jd"
+#define PRIiMAX "ji"
+#define PRIuMAX "ju"
+#define PRIoMAX "jo"
+#define PRIxMAX "jx"
+#define PRIXMAX "jX"
+
 #endif /* INTTYPES_H */
diff --git a/src/include/stdint.h b/src/include/stdint.h
index f363aab..0a8e153 100644
--- a/src/include/stdint.h
+++ b/src/include/stdint.h
@@ -110,10 +110,4 @@
 #define true    1
 #define false   0

-/* TODO: move into inttypes.h */
-#ifndef __ROMCC__
-#define PRIu64  "llu"
-#define PRIxPTR "lx"
-#endif
-
 #endif /* STDINT_H */
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c
index adacf6b..1319b86 100644
--- a/src/lib/timestamp.c
+++ b/src/lib/timestamp.c
@@ -180,8 +180,7 @@
    tse->entry_stamp = ts_time - ts_table->base_time;

if (CONFIG(TIMESTAMPS_ON_CONSOLE))
-		printk(BIOS_SPEW, "Timestamp - %s: %" PRIu64 "\n",
-				timestamp_name(id), ts_time);
+		printk(BIOS_SPEW, "Timestamp - %s: %llu\n", timestamp_name(id), ts_time);

if (ts_table->num_entries == ts_table->max_entries)
    	printk(BIOS_ERR, "ERROR: Timestamp table full\n");