git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/3] introduce "format" date-mode

From
Jeff King <peff@peff.net>
Date
Jun 30, 2015, 17:58 UTC
Message-ID
<20150630175852.GB5349@peff.net>
In-Reply-To
<CAPig+cTXc_RXbOAOaF2MFjrg+DSet=g0XQMZY0ErMYAmNVSV+g@mail.gmail.com>
On Tue, Jun 30, 2015 at 12:58:33PM -0400, Eric Sunshine wrote:
Show 14 quoted lines
> > Basically I was trying to avoid making any assumptions about exactly how
> > strftime works. But presumably "stick a space in the format" is a
> > universally reasonable thing to do. It's a hack, but it's contained to
> > the function.
> 
> I don't think we're making any assumptions about strftime(). POSIX states:
> 
>     The format string consists of zero or more conversion
>     specifications and ordinary characters. [...] All ordinary
>     characters (including the terminating NUL character) are copied
>     unchanged into the array.
> 
> So, we seem to be on solid footing with this approach (even though
> it's a localized hack).

Yeah, sorry I wasn't more clear. I had originally been thinking of making assumptions like "well, %c cannot ever be blank". But your solution does not suffer from that level of knowledge. I think it is reasonably clever.

Show 7 quoted lines
> Yeah, I toyed with the idea of increasing the requested amount each
> iteration but wanted to keep the example simple, thus left it out.
> However, for some reason, I was thinking that strbuf_grow() was
> unconditionally expanding the buffer by the requested amount rather
> than merely ensuring that that amount was availabile, so the amount
> clearly needs to be increased on each iteration. Thanks for pointing
> that out.

FWIW, I had to look at it to double-check. I've often made the same mistake.

> Beyond the extra allocation, I was also concerned about the
> sledgehammer approach of "%s " to append a single character when there
> are much less expensive ways to do so.

I don't think there's any other way. We have to feed a contiguous buffer to strftime, and we don't own the buffer, so we have to make a new copy.

-Peff
Previous: Eric SunshineNext: Eric Sunshine
Message 22 of 31 in “several date related issues”
  1. H.Merijn BrandJun 25, 2015
  2. Jeff KingJun 25, 2015
  3. H.Merijn BrandJun 25, 2015
  4. 0/3 localized date formatJeff King, Jun 25, 2015
  5. 1/3 show-branch: use DATE_RELATIVE instead of magic numberJeff King, Jun 25, 2015
  6. 2/3 convert "enum date_mode" into a structJeff King, Jun 25, 2015
  7. John KeepingJun 25, 2015
  8. Jeff KingJun 25, 2015
  9. Junio C HamanoJul 7, 2015
  10. Jeff KingJul 7, 2015
  11. Junio C HamanoJul 7, 2015
  12. Jeff KingJul 7, 2015
  13. Junio C HamanoJul 7, 2015
  14. 3/3 introduce "format" date-modeJeff King, Jun 25, 2015
  15. Eric SunshineJun 29, 2015
  16. Jeff KingJun 30, 2015
  17. Junio C HamanoJun 30, 2015
  18. Jeff KingJun 30, 2015
  19. Junio C HamanoJun 30, 2015
  20. Jeff KingJun 30, 2015
  21. Eric SunshineJun 30, 2015
  22. Jeff KingJun 30, 2015
  23. Eric SunshineJun 30, 2015
  24. Jeff KingJun 30, 2015
  25. Junio C HamanoJun 30, 2015
  26. Eric SunshineJun 30, 2015
  27. Jeff KingJun 30, 2015
  28. Jeff KingJun 30, 2015
  29. Eric SunshineJun 30, 2015
  30. Eric SunshineJul 21, 2015
  31. Jeff KingJul 21, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.