| Message ID | e8c57268455291bad0bbcdf0013b0d5aa349be1b.1611273359.git.bobbyeshleman@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support Secure Boot for multiboot2 Xen | expand |
On 22.01.2021 01:51, Bobby Eshleman wrote: > From: Daniel Kiper <daniel.kiper@oracle.com> > > POSIX time is required to fill the TimeDateStamp field > in the PE header. > > Use SOURCE_DATE_EPOCH if available, otherwise use > `date +%s` (available on both Linux and FreeBSD). > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > Signed-off-by: Bobby Eshleman <bobbyeshleman@gmail.com> Especially with there not being any user of the new item, you will want to at least briefly explain ... > --- a/xen/Makefile > +++ b/xen/Makefile > @@ -11,6 +11,7 @@ export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) | > export XEN_BUILD_DATE ?= $(shell LC_ALL=C date) > export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T) > export XEN_BUILD_HOST ?= $(shell hostname) > +export XEN_BUILD_POSIX_TIME ?= $(shell echo $${SOURCE_DATE_EPOCH:-$(shell date +%s)}) ... the use of SOURCE_DATE_EPOCH here when it's not used for XEN_BUILD_TIME (the two could also do with living side by side) and ... > --- a/xen/include/xen/compile.h.in > +++ b/xen/include/xen/compile.h.in > @@ -1,5 +1,6 @@ > #define XEN_COMPILE_DATE "@@date@@" > #define XEN_COMPILE_TIME "@@time@@" > +#define XEN_COMPILE_POSIX_TIME @@posix_time@@ > #define XEN_COMPILE_BY "@@whoami@@" > #define XEN_COMPILE_DOMAIN "@@domain@@" > #define XEN_COMPILE_HOST "@@hostname@@" ... the lack of quotes here when all neighboring items have them. Jan
On Fri, Jan 22, 2021 at 12:27:29PM +0100, Jan Beulich wrote: > On 22.01.2021 01:51, Bobby Eshleman wrote: > > export XEN_BUILD_DATE ?= $(shell LC_ALL=C date) > > export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T) > > export XEN_BUILD_HOST ?= $(shell hostname) > > +export XEN_BUILD_POSIX_TIME ?= $(shell echo $${SOURCE_DATE_EPOCH:-$(shell date +%s)}) > > ... the use of SOURCE_DATE_EPOCH here when it's not used for > XEN_BUILD_TIME (the two could also do with living side by > side) and ... > XEN_BUILD_TIME is of the form "HH:MM:SS" and SOURCE_DATE_EPOCH / date +%s are unix timestamps (seconds since epoch). On Linux, `date -d` could be used to equalize the two timestamps... I'm not sure about FreeBSD, as -d is not required by POSIX. I could place them side-by-side if that's preferred. I placed it afterwards here so that there wasn't one oddly aligned "?=" assignment in the middle of the others, as in rev2 it was requested their alignment be retained here. > > --- a/xen/include/xen/compile.h.in > > +++ b/xen/include/xen/compile.h.in > > @@ -1,5 +1,6 @@ > > #define XEN_COMPILE_DATE "@@date@@" > > #define XEN_COMPILE_TIME "@@time@@" > > +#define XEN_COMPILE_POSIX_TIME @@posix_time@@ > > #define XEN_COMPILE_BY "@@whoami@@" > > #define XEN_COMPILE_DOMAIN "@@domain@@" > > #define XEN_COMPILE_HOST "@@hostname@@" > > ... the lack of quotes here when all neighboring items have > them. > XEN_COMPILE_POSIX_TIME is used as a long, while the others are used as strings. Should this be commented? Thank you for the feedback. Best, Bobby
On 22.01.2021 22:57, Bobby Eshleman wrote: > On Fri, Jan 22, 2021 at 12:27:29PM +0100, Jan Beulich wrote: >> On 22.01.2021 01:51, Bobby Eshleman wrote: >>> export XEN_BUILD_DATE ?= $(shell LC_ALL=C date) >>> export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T) >>> export XEN_BUILD_HOST ?= $(shell hostname) >>> +export XEN_BUILD_POSIX_TIME ?= $(shell echo $${SOURCE_DATE_EPOCH:-$(shell date +%s)}) >> >> ... the use of SOURCE_DATE_EPOCH here when it's not used for >> XEN_BUILD_TIME (the two could also do with living side by >> side) and ... >> > > XEN_BUILD_TIME is of the form "HH:MM:SS" and SOURCE_DATE_EPOCH / date > +%s are unix timestamps (seconds since epoch). On Linux, `date -d` > could be used to equalize the two timestamps... I'm not sure about > FreeBSD, as -d is not required by POSIX. > > I could place them side-by-side if that's preferred. I placed it > afterwards here so that there wasn't one oddly aligned "?=" assignment > in the middle of the others, as in rev2 it was requested their alignment > be retained here. Personally I'd prefer if the time ones were all together. The odd alignment isn't uncommon when introducing new items with unexpectedly long names into a previously aligned list of items. Of course you will want to drop the excess blanks in any event - one each suffice as separators. >>> --- a/xen/include/xen/compile.h.in >>> +++ b/xen/include/xen/compile.h.in >>> @@ -1,5 +1,6 @@ >>> #define XEN_COMPILE_DATE "@@date@@" >>> #define XEN_COMPILE_TIME "@@time@@" >>> +#define XEN_COMPILE_POSIX_TIME @@posix_time@@ >>> #define XEN_COMPILE_BY "@@whoami@@" >>> #define XEN_COMPILE_DOMAIN "@@domain@@" >>> #define XEN_COMPILE_HOST "@@hostname@@" >> >> ... the lack of quotes here when all neighboring items have >> them. >> > > XEN_COMPILE_POSIX_TIME is used as a long, while the others are used as > strings. Should this be commented? Not sure about commenting, but at the very least the description will imo want to point out the (intentional) difference. You shouldn't imply readers know in advance what a subsequent patch may use this for. Jan
diff --git a/xen/Makefile b/xen/Makefile index 544cc0995d..85f9b763a4 100644 --- a/xen/Makefile +++ b/xen/Makefile @@ -11,6 +11,7 @@ export XEN_DOMAIN ?= $(shell ([ -x /bin/dnsdomainname ] && /bin/dnsdomainname) | export XEN_BUILD_DATE ?= $(shell LC_ALL=C date) export XEN_BUILD_TIME ?= $(shell LC_ALL=C date +%T) export XEN_BUILD_HOST ?= $(shell hostname) +export XEN_BUILD_POSIX_TIME ?= $(shell echo $${SOURCE_DATE_EPOCH:-$(shell date +%s)}) # Best effort attempt to find a python interpreter, defaulting to Python 3 if # available. Fall back to just `python` if `which` is nowhere to be found. @@ -386,6 +387,7 @@ delete-unfresh-files: include/xen/compile.h: include/xen/compile.h.in .banner @sed -e 's/@@date@@/$(XEN_BUILD_DATE)/g' \ -e 's/@@time@@/$(XEN_BUILD_TIME)/g' \ + -e 's/@@posix_time@@/$(XEN_BUILD_POSIX_TIME)/g' \ -e 's/@@whoami@@/$(XEN_WHOAMI)/g' \ -e 's/@@domain@@/$(XEN_DOMAIN)/g' \ -e 's/@@hostname@@/$(XEN_BUILD_HOST)/g' \ diff --git a/xen/include/xen/compile.h.in b/xen/include/xen/compile.h.in index 440ecb25c1..b2ae6f96ad 100644 --- a/xen/include/xen/compile.h.in +++ b/xen/include/xen/compile.h.in @@ -1,5 +1,6 @@ #define XEN_COMPILE_DATE "@@date@@" #define XEN_COMPILE_TIME "@@time@@" +#define XEN_COMPILE_POSIX_TIME @@posix_time@@ #define XEN_COMPILE_BY "@@whoami@@" #define XEN_COMPILE_DOMAIN "@@domain@@" #define XEN_COMPILE_HOST "@@hostname@@"