[v4,00/27] Compile with `-Wwrite-strings`

Message

Patrick Steinhardt June 4, 2024, 12:36 p.m. UTC

Hi,

this is another minor reroll of my patch series that prepares the code
base to compile with `-Wwrite-strings`.

There are only some small changes compared to v3:

  - Fix typos in some commit messages.

  - Fix accidental conversion of "." to "" in basename/dirname
    compatibility code. Also, remove the unused variable.

Thanks for your reviews!

Patrick

Patrick Steinhardt (27):
  global: improve const correctness when assigning string constants
  global: convert intentionally-leaking config strings to consts
  refs/reftable: stop micro-optimizing refname allocations on copy
  reftable: cast away constness when assigning constants to records
  refspec: remove global tag refspec structure
  builtin/remote: cast away constness in `get_head_names()`
  diff: cast string constant in `fill_textconv()`
  line-log: stop assigning string constant to file parent buffer
  line-log: always allocate the output prefix
  entry: refactor how we remove items for delayed checkouts
  ident: add casts for fallback name and GECOS
  object-file: mark cached object buffers as const
  object-file: make `buf` parameter of `index_mem()` a constant
  pretty: add casts for decoration option pointers
  compat/win32: fix const-correctness with string constants
  http: do not assign string constant to non-const field
  parse-options: cast long name for OPTION_ALIAS
  send-pack: always allocate receive status
  remote-curl: avoid assigning string constant to non-const variable
  revision: always store allocated strings in output encoding
  mailmap: always store allocated strings in mailmap blob
  imap-send: drop global `imap_server_conf` variable
  imap-send: fix leaking memory in `imap_server_conf`
  builtin/rebase: do not assign default backend to non-constant field
  builtin/rebase: always store allocated string in `options.strategy`
  builtin/merge: always store allocated strings in `pull_twohead`
  config.mak.dev: enable `-Wwrite-strings` warning

 builtin/bisect.c             |   3 +-
 builtin/blame.c              |   2 +-
 builtin/bugreport.c          |   2 +-
 builtin/check-ignore.c       |   4 +-
 builtin/clone.c              |  14 ++--
 builtin/commit.c             |   6 +-
 builtin/diagnose.c           |   2 +-
 builtin/fetch.c              |  11 ++-
 builtin/log.c                |   2 +-
 builtin/mailsplit.c          |   4 +-
 builtin/merge.c              |  18 +++--
 builtin/pull.c               |  52 +++++++-------
 builtin/rebase.c             |  81 ++++++++++++----------
 builtin/receive-pack.c       |   4 +-
 builtin/remote.c             |  12 ++--
 builtin/revert.c             |   2 +-
 builtin/send-pack.c          |   2 +
 compat/basename.c            |  16 ++++-
 compat/mingw.c               |  28 ++++----
 compat/regex/regcomp.c       |   2 +-
 compat/winansi.c             |   2 +-
 config.mak.dev               |   1 +
 diff.c                       |   6 +-
 diffcore-rename.c            |   6 +-
 entry.c                      |  14 ++--
 fmt-merge-msg.c              |   2 +-
 fsck.c                       |   2 +-
 fsck.h                       |   2 +-
 gpg-interface.c              |   6 +-
 http-backend.c               |   2 +-
 http.c                       |   5 +-
 ident.c                      |   4 +-
 imap-send.c                  | 130 ++++++++++++++++++++---------------
 line-log.c                   |  22 +++---
 mailmap.c                    |   2 +-
 merge-ll.c                   |  11 ++-
 object-file.c                |  23 ++++---
 parse-options.h              |   2 +-
 pretty.c                     |   6 +-
 refs.c                       |   2 +-
 refs.h                       |   2 +-
 refs/reftable-backend.c      |  28 ++++----
 refspec.c                    |  13 ----
 refspec.h                    |   1 -
 reftable/basics.c            |  15 ++--
 reftable/basics.h            |   4 +-
 reftable/basics_test.c       |   4 +-
 reftable/block_test.c        |   2 +-
 reftable/merged_test.c       |  44 ++++++------
 reftable/readwrite_test.c    |  32 ++++-----
 reftable/record.c            |   6 +-
 reftable/stack.c             |  10 +--
 reftable/stack_test.c        |  56 +++++++--------
 remote-curl.c                |  53 +++++++-------
 revision.c                   |   3 +-
 run-command.c                |   2 +-
 send-pack.c                  |   2 +-
 t/helper/test-hashmap.c      |   3 +-
 t/helper/test-json-writer.c  |  10 +--
 t/helper/test-regex.c        |   4 +-
 t/helper/test-rot13-filter.c |   5 +-
 t/t3900-i18n-commit.sh       |   1 +
 t/t3901-i18n-patch.sh        |   1 +
 t/unit-tests/t-strbuf.c      |  10 +--
 trailer.c                    |   2 +-
 userdiff.c                   |  10 +--
 userdiff.h                   |  12 ++--
 wt-status.c                  |   2 +-
 68 files changed, 468 insertions(+), 386 deletions(-)

Range-diff against v3:
 1:  e01fde88fe =  1:  e01fde88fe global: improve const correctness when assigning string constants
 2:  92cb0b28c6 =  2:  92cb0b28c6 global: convert intentionally-leaking config strings to consts
 3:  085d90c8b6 !  3:  379145478c refs/reftable: stop micro-optimizing refname allocations on copy
    @@ Commit message
         refs/reftable: stop micro-optimizing refname allocations on copy
     
         When copying refs, we execute `write_copy_table()` to write the new
    -    table. As the names arge given to use via `arg->newname` and
    +    table. As the names are given to us via `arg->newname` and
         `arg->oldname`, respectively, we optimize away some allocations by
    -    assigning those fields to the reftable records we are about to write.
    -    This requires us to cast the input to `char *` pointers as they are in
    -    fact constant strings. Later on, we then unset the refname for all of
    -    the records before calling `reftable_log_record_release()` on them.
    +    assigning those fields to the reftable records we are about to write
    +    directly, without duplicating them. This requires us to cast the input
    +    to `char *` pointers as they are in fact constant strings. Later on, we
    +    then unset the refname for all of the records before calling
    +    `reftable_log_record_release()` on them.
     
         We also do this when assigning the "HEAD" constant, but here we do not
         cast because its type is `char[]` by default. It's about to be turned
 4:  8692d9d9af =  4:  d0a2a2f6c5 reftable: cast away constness when assigning constants to records
 5:  db4d062014 !  5:  ead27d3d97 refspec: remove global tag refspec structure
    @@ Commit message
         refspec: remove global tag refspec structure
     
         We have a global tag refspec structure that is used by both git-clone(1)
    -    and git-fetch(1). Initialization fo the structure will break once we
    +    and git-fetch(1). Initialization of the structure will break once we
         enable `-Wwrite-strings`, even though the breakage is harmless. While we
         could just add casts, the structure isn't really required in the first
         place as we can simply initialize the structures at the respective
 6:  6a3c8d351e =  6:  7cb5df9182 builtin/remote: cast away constness in `get_head_names()`
 7:  750429472a =  7:  6e631a9ea4 diff: cast string constant in `fill_textconv()`
 8:  cc8fa1896d =  8:  ac164651a3 line-log: stop assigning string constant to file parent buffer
 9:  03dbdd235b =  9:  b717af02f0 line-log: always allocate the output prefix
10:  6fcb7d6685 = 10:  b46dd3210d entry: refactor how we remove items for delayed checkouts
11:  81e20a7bb8 = 11:  030dbd0288 ident: add casts for fallback name and GECOS
12:  384b4c8967 = 12:  ecca8e973d object-file: mark cached object buffers as const
13:  a1e8e77641 = 13:  62f0e47f94 object-file: make `buf` parameter of `index_mem()` a constant
14:  4d95abe9cc = 14:  e057ead2b4 pretty: add casts for decoration option pointers
15:  3d92528125 ! 15:  06b6120d26 compat/win32: fix const-correctness with string constants
    @@ Commit message
         Signed-off-by: Patrick Steinhardt <[e-mail]>
     
      ## compat/basename.c ##
    -@@
    - #include "../git-compat-util.h"
    - #include "../strbuf.h"
    - 
    -+static char current_directory[] = ".";
    -+
    - /* Adapted from libiberty's basename.c.  */
    - char *gitbasename (char *path)
    - {
     @@ compat/basename.c: char *gitbasename (char *path)
      		skip_dos_drive_prefix(&path);
      
    @@ compat/basename.c: char *gitbasename (char *path)
     +		 * pointer to internal memory at times. The cast is a result of
     +		 * that.
     +		 */
    -+		return (char *) "";
    ++		return (char *) ".";
      
      	for (base = path; *path; path++) {
      		if (!is_dir_sep(*path))
    @@ compat/basename.c: char *gitdirname(char *path)
     +		 * pointer to internal memory at times. The cast is a result of
     +		 * that.
     +		 */
    -+		return (char *) "";
    ++		return (char *) ".";
      
      	if ((dos_drive_prefix = skip_dos_drive_prefix(&p)) && !*p)
      		goto dot;
16:  8a98078439 = 16:  a8ef39d73d http: do not assign string constant to non-const field
17:  26c4c76c86 = 17:  9d596a07c5 parse-options: cast long name for OPTION_ALIAS
18:  e3227bd3f3 = 18:  4019b532f9 send-pack: always allocate receive status
19:  af82e49682 = 19:  f2f1ada143 remote-curl: avoid assigning string constant to non-const variable
20:  077f10d0dc = 20:  27660b908c revision: always store allocated strings in output encoding
21:  fb240598b4 = 21:  ef43c1b18f mailmap: always store allocated strings in mailmap blob
22:  291030faa7 = 22:  0a69ce4b44 imap-send: drop global `imap_server_conf` variable
23:  9b376a313f = 23:  9ccafd286b imap-send: fix leaking memory in `imap_server_conf`
24:  0a84d66e68 = 24:  e19457d20c builtin/rebase: do not assign default backend to non-constant field
25:  5c8bee3695 = 25:  f548241960 builtin/rebase: always store allocated string in `options.strategy`
26:  84c0149c8f = 26:  78ac075644 builtin/merge: always store allocated strings in `pull_twohead`
27:  3427547134 = 27:  0cd4ce07d8 config.mak.dev: enable `-Wwrite-strings` warning