Re: [PATCH v5 2/2] strbuf: change an always NULL/"" strbuf_addftime() param to bool

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 24.06.2017 um 14:14 schrieb Ævar Arnfjörð Bjarmason:
Change the code for deciding what's to be done about %Z to stop
passing always either a NULL or "" char * to
strbuf_addftime(). Instead pass a boolean int to indicate whether the
strftime() %Z format should be suppressed by converting it to an empty
string, which is what this code is actually doing.

This code grew organically between the changes in 9eafe86d58 ("Merge
branch 'rs/strbuf-addftime-zZ'", 2017-06-22). The intent was to use
this API in the future to pass a custom leave the door open to pass a
custom timezone name to the function (see my [1] and related
messages).

"leave the door open to pass a" seems redundant.

But that's not what this code does now, and this strbuf_addstr() call
always being redundant makes it hard to understand the current
functionality. So simplify this internal API to match its use, we can
always change it in the future if it gets a different use-case.

I don't understand the confusion, but of course I'm biased. And I don't
like binary parameters in general and would use named flags or two
function names in most cases.  But that aside I find the description
hard to follow (perhaps I should do something about my attention span).

Here's an attempt at a commit message that would have be easier to
understand for me:

  strbuf_addstr() allows callers to pass a time zone name for expanding
  %Z.  The only current caller either passes the empty string or NULL,
  in which case %Z is handed over verbatim to strftime(3).  Replace that
  string parameter with a flag controlling whether to remove %Z from the
  format specification.  This simplifies the code.

René



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux