All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Bobby Eshleman <bobbyeshleman@gmail.com>
Cc: Daniel Kiper <daniel.kiper@oracle.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Ian Jackson <iwj@xenproject.org>, Julien Grall <julien@xen.org>,
	Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v3 1/5] xen: add XEN_BUILD_POSIX_TIME
Date: Mon, 25 Jan 2021 09:58:28 +0100	[thread overview]
Message-ID: <b20d3e47-6872-d885-c1fd-fe8efdf25ce6@suse.com> (raw)
In-Reply-To: <YAtKZ3QiE8ZNdNif@piano>

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


  reply	other threads:[~2021-01-25  8:58 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-22  0:51 [PATCH v3 0/5] Support Secure Boot for multiboot2 Xen Bobby Eshleman
2021-01-22  0:51 ` [PATCH v3 1/5] xen: add XEN_BUILD_POSIX_TIME Bobby Eshleman
2021-01-22 11:27   ` Jan Beulich
2021-01-22 21:57     ` Bobby Eshleman
2021-01-25  8:58       ` Jan Beulich [this message]
2021-01-22  0:51 ` [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary Bobby Eshleman
2021-03-15 13:36   ` Jan Beulich
2021-05-07 20:26     ` Bob Eshleman
2021-05-17  6:48       ` Jan Beulich
2021-05-17 13:20         ` Daniel Kiper
2021-05-17 13:24           ` Jan Beulich
2021-05-18 17:46             ` Daniel Kiper
2021-05-19  9:29               ` Jan Beulich
2021-05-19 12:48                 ` Daniel Kiper
2021-05-19 14:35                   ` Jan Beulich
2021-06-09 13:18                     ` Daniel Kiper
2021-06-09 13:45                       ` Jan Beulich
2021-01-22  0:51 ` [PATCH v3 3/5] xen/x86: add some addresses to the Multiboot header Bobby Eshleman
2021-03-15 15:05   ` Jan Beulich
2021-01-22  0:51 ` [PATCH v3 4/5] xen/x86: add some addresses to the Multiboot2 header Bobby Eshleman
2021-02-23  9:04   ` Roger Pau Monné
2021-02-23 18:07     ` Bob Eshleman
2021-01-22  0:51 ` [PATCH v3 5/5] xen/x86/efi: Verify dom0 kernel with SHIM_LOCK protocol in efi_multiboot2() Bobby Eshleman
2021-03-16 15:08   ` Jan Beulich
2021-01-22  9:39 ` [PATCH v3 0/5] Support Secure Boot for multiboot2 Xen Jan Beulich
2021-01-22 21:18   ` Bobby Eshleman
2021-01-25  8:52     ` Jan Beulich
2021-02-22 18:04 ` Bobby Eshleman
2021-02-23  7:16   ` Jan Beulich
2021-02-23 18:00     ` Bob Eshleman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b20d3e47-6872-d885-c1fd-fe8efdf25ce6@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bobbyeshleman@gmail.com \
    --cc=daniel.kiper@oracle.com \
    --cc=george.dunlap@citrix.com \
    --cc=iwj@xenproject.org \
    --cc=julien@xen.org \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.