Re: CVS commit: src/external/bsd/ntp/lib/libntp
On Fri, Apr 19, 2024 at 10:48:10PM +0700, Robert Elz wrote:
> Date:Fri, 19 Apr 2024 14:58:18 +
> From:"Jonathan A. Kollasch"
> Message-ID: <20240419145818.351d2f...@cvs.netbsd.org>
>
> | - bail out if resulting __DATE__/__TIME__ replacement strings are empty
>
> If you want to do that (not that it would be useful, even if the %b
> (etc) conversions produced nothing, there would still be two spaces
> in the output. It is almost impossible to get date to exit with an
> error code (and nothing on stdout) in cases like this.
>
> But if this is worth keeping, then
>
> if ${MKREPRO_TIME} == "" || ${MKREPRO_TIME} == ""
>
> probably is not what you wanted.
I've tested this with MKREPRO_TIMESTAMP=RandomNonNumericString, and
MKREPRO_TIME/MKREPRO_DATE came out "" when nbdate produced a non-zero
exit status and a warning message.
I wish nbmake could actually error when it gets a non-zero exit status
there, but it merely spouts a warning and continues.
I've spent enough time looking at this.
Re: CVS commit: src/external/bsd/ntp/lib/libntp
Date:Fri, 19 Apr 2024 14:58:18 +
From:"Jonathan A. Kollasch"
Message-ID: <20240419145818.351d2f...@cvs.netbsd.org>
| - bail out if resulting __DATE__/__TIME__ replacement strings are empty
If you want to do that (not that it would be useful, even if the %b
(etc) conversions produced nothing, there would still be two spaces
in the output. It is almost impossible to get date to exit with an
error code (and nothing on stdout) in cases like this.
But if this is worth keeping, then
if ${MKREPRO_TIME} == "" || ${MKREPRO_TIME} == ""
probably is not what you wanted.
kre
Re: CVS commit: src/external/bsd/ntp/lib/libntp
On Fri 19 Apr 2024 at 14:47:23 +0200, Martin Husemann wrote: > The problem (IIUC) is that the ntp code parses this value internally, so > it won't talk to anyone claiming a time before the time ntpd was compiled > (or something along that line). Would it make sense to just supply a fixed string to effectively defang that code? Or just rip it out completely. It sounds like a giant footgun somehow. -Olaf. -- ___ Olaf 'Rhialto' Seibert \X/ There is no AI. There is just someone else's work. --I. Rose signature.asc Description: PGP signature
Re: CVS commit: src/external/bsd/ntp/lib/libntp
Date:Fri, 19 Apr 2024 14:57:46 +0200 From:Martin Husemann Message-ID: <20240419125746.gb26...@mail.duskware.de> | On Fri, Apr 19, 2024 at 02:47:23PM +0200, Martin Husemann wrote: | > The commit message could have explained that a bit. | | And it actually should have been "+%b %e %Y" - from the C18 standard draft: | | 6.10.8.1 Mandatory macros | | __DATE__ | | The date of translation of the preprocessing translation unit: a | character string literal of the form "Mmm dd ", where the names of | the months are the same as those generated by the asctime function, If that's still in the most recent C (in that form anyway) I'd be surprised, as asctime() is more or less deprecated I think. But if that is what is required, then the TOOL_DATE needs to be run with LC_ALL=C (or LC_ALL=POSIX) so that you get the month name in English, rather than whatever the local locale's short name for the month would be. It is still a stupid format... kre
Re: CVS commit: src/external/bsd/ntp/lib/libntp
On Fri, Apr 19, 2024 at 02:47:23PM +0200, Martin Husemann wrote: > The commit message could have explained that a bit. And it actually should have been "+%b %e %Y" - from the C18 standard draft: 6.10.8.1 Mandatory macros __DATE__ The date of translation of the preprocessing translation unit: a character string literal of the form "Mmm dd ", where the names of the months are the same as those generated by the asctime function, and the first character of dd is a space character if the value is less than 10. If the date of translation is not available, an implementation-defined valid date shall be supplied. Martin
Re: CVS commit: src/external/bsd/ntp/lib/libntp
On Fri, Apr 19, 2024 at 04:36:46PM +0700, Robert Elz wrote:
> I don't understand that change, it altered from
>
> MKREPRO_DATE != ${TOOL_DATE} -u -r "${MKREPRO_TIMESTAMP}" "+%F"
> to
> MKREPRO_DATE != ${TOOL_DATE} -u -r "${MKREPRO_TIMESTAMP}" "+%b %d %Y"
>
> %F is simply a shorthand for %Y-%m-%d (which is the correct way to write
> locale independent dates), %b is a locale dependent name of the month, and
> what's more this is in US-centric mon day year format, which we should avoid
> (aside from anything else, it is a dumb format, with the smallest value unit
> sitting in the middle, rather than at one end or the other).
The problem (IIUC) is that the ntp code parses this value internally, so
it won't talk to anyone claiming a time before the time ntpd was compiled
(or something along that line).
And __DATE__ gives us a stupid format:
> echo __DATE__ | cc -E - | tail -1
"Apr 19 2024"
which is different from
> date -u -r 1713530399 "+%F"
2024-04-19
but matches
> date -u -r 1713530399 "+%b %d %Y"
Apr 19 2024
The commit message could have explained that a bit.
Martin
Re: CVS commit: src/external/bsd/ntp/lib/libntp
Date:Thu, 18 Apr 2024 19:23:54 +
From:"Jonathan A. Kollasch"
Message-ID: <20240418192354.1bcc1f...@cvs.netbsd.org>
| Module Name:src
| Committed By: jakllsch
| Date: Thu Apr 18 19:23:54 UTC 2024
|
| Modified Files:
| src/external/bsd/ntp/lib/libntp: Makefile
|
| Log Message:
| Format MKREPRO_TIMESTAMP with "%b %d %Y" to correctly substitute __DATE__
I don't understand that change, it altered from
MKREPRO_DATE != ${TOOL_DATE} -u -r "${MKREPRO_TIMESTAMP}" "+%F"
to
MKREPRO_DATE != ${TOOL_DATE} -u -r "${MKREPRO_TIMESTAMP}" "+%b %d %Y"
%F is simply a shorthand for %Y-%m-%d (which is the correct way to write
locale independent dates), %b is a locale dependent name of the month, and
what's more this is in US-centric mon day year format, which we should avoid
(aside from anything else, it is a dumb format, with the smallest value unit
sitting in the middle, rather than at one end or the other).
If there is a problem where support for %F is missing somewhere, (and if
that happens in TOOL_DATE it must mean that we're using a locale strftime()
when building it, in which case we should add our version to the library)
please replace it with %Y-%m-%d (which is what it is defined to be) instead
of anything using %b (or %a or %B or %A) (or any other weird formats).
Further, if some format like that was needed for some reason, then it
probably should use %e rather than %d (to suppress the leading 0 on dates
before the 10th). But don't just do that either.
If there isn't an actual problem using %F, please just revert this.
And lastly, and this applies to everyone - please do not request
pullups of anything (except perhaps urgent security fixes) immediately
after the change is committed - give it at least a few days, in case of
objection, breakage, ...
kre
