Skip to content

writeout: add %time{}#18119

Closed
bagder wants to merge 11 commits into
masterfrom
bagder/writeout-time
Closed

writeout: add %time{}#18119
bagder wants to merge 11 commits into
masterfrom
bagder/writeout-time

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Jul 31, 2025

Output the current UTC time using strftime format. %f is an extra curl specific flag to output the microsecond fraction of the current second.

TODO

  • decide if we want this at all
  • is the strftime good enough?
  • add test case to verify functionality
  • %z works in a portable fashion
  • %Z works in a portable fashion
  • check curl -h -w with this PR as it shows a formatting error not seen in the man page => managen: reset text mode at end of table marker #18139

@dfandrich
Copy link
Copy Markdown
Contributor

dfandrich commented Jul 31, 2025 via email

@github-actions github-actions Bot added the tests label Jul 31, 2025
@bagder bagder force-pushed the bagder/writeout-time branch from da6d1fa to ce6d408 Compare August 1, 2025 08:39
Output the current UTC time using strftime format. %f is an extra curl
specific flag to output the microsecond fraction of the current second.

Verified by test 1981
@bagder bagder force-pushed the bagder/writeout-time branch from ce6d408 to 530e327 Compare August 1, 2025 08:51
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Aug 1, 2025

Some of our compiler warnings sometimes are quite counter productive:

../../src/tool_writeout.c:575:23: error: conversion from 'long long int' to 'long int' may change value [-Werror=conversion]
  575 |         cnow.tv_sec = (time_t)val;
      |                       ^

On the line above 575, we make sure that the value stored in val cannot be larger than what fits in a time_t...

This reverts commit db19158.
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Aug 1, 2025

Is there a better solution than to switch off those warnings (for this code)?

@bagder bagder marked this pull request as ready for review August 1, 2025 12:33
@icing
Copy link
Copy Markdown
Contributor

icing commented Aug 1, 2025

Is there a better solution than to switch off those warnings (for this code)?

Disable debug builds? 😬

have you tried (val < TIME_T_MAX) ? (time_t)val : TIME_T_MAX;?

@bagder
Copy link
Copy Markdown
Member Author

bagder commented Aug 1, 2025

Grr, mingw:

struct timeval
{
	long tv_sec;
	long tv_usec;
};

@icing
Copy link
Copy Markdown
Contributor

icing commented Aug 2, 2025

So, if I understand the approach correctly here, %time{fmt} will output the current time at the moment the writeout happens. Which usually is the end of the transfer. Usually, except when there is more than one transfer and other do some time consuming stuff (like a FTP blocking data connect).

Do I read this right?

Before looking at the details here, I was expecting to have a %start-time or so, that gives the timestamp of a transfer related event (like the start, could also be the end, whatever). And that timestamp to be in a fixed format, like rfc3339.

Can you explain what the advantages of the current approach and the repetition of strftime()? It's very flexible, but is this a good thing, I wonder?

@bagder
Copy link
Copy Markdown
Member Author

bagder commented Aug 3, 2025

So, if I understand the approach correctly here, %time{fmt} will output the current time at the moment the writeout happens. Which usually is the end of the transfer. Usually, except when there is more than one transfer and other do some time consuming stuff (like a FTP blocking data connect).

It is still after the transfer was just completed. yes.

Before looking at the details here, I was expecting to have a %start-time or so, that gives the timestamp of a transfer related event (like the start, could also be the end, whatever). And that timestamp to be in a fixed format, like rfc3339.

I think it is fair to consider a start-time as well, sure. But as my primary use-case here is for logging the transfer, I believe the end time is more useful .

Can you explain what the advantages of the current approach and the repetition of strftime()? It's very flexible, but is this a good thing, I wonder?

My primary use case is when this is used for logging (I know a real user wanting this). Basically doing curl $URL -w $FORMAT >> logfile - and since there are many different logfile formats and people want different things for their timestamps, I figured it was easy enough to offer that flexibility rather than picking a single format that is bound to end up the wrong one for some users.

@bagder bagder closed this in fadc487 Aug 4, 2025
@bagder bagder deleted the bagder/writeout-time branch August 4, 2025 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants