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

[PATCH v4 0/5] repo-settings.c: refactor for clarity, get rid of hacks etc.

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Sep 21, 2021, 13:12 UTC
Message-ID
<cover-v4-0.5-00000000000-20210921T131003Z-avarab@gmail.com>
In-Reply-To
<cover-v3-0.5-00000000000-20210919T084703Z-avarab@gmail.com>

A hopefully final re-roll addressing Taylor's v3 review, except for the suggestion (that I read as) perhaps retaining the test-only code, which I've decided not to do per http://lore.kernel.org/git/87tuieakms.fsf@evledraar.gmail.com

The x(un)setenv() now returns void, and the error messages are less chatty, I also improved a BUG() message in 4/5 that we end up deleting in 5/5 anyway, so it doesn't matter for the end-state, just for understanding the patches.

Ævar Arnfjörð Bjarmason (5):
  wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c
  environment.c: remove test-specific "ignore_untracked..." variable
  read-cache & fetch-negotiator: check "enum" values in switch()
  repo-settings.c: simplify the setup
  repository.h: don't use a mix of int and bitfields
 cache.h                              |   7 --
 environment.c                        |  10 +--
 fetch-negotiator.c                   |   1 -
 git-compat-util.h                    |   2 +
 read-cache.c                         |  19 +++--
 repo-settings.c                      | 115 +++++++++++++--------------
 repository.h                         |  20 ++---
 t/helper/test-dump-untracked-cache.c |   6 +-
 wrapper.c                            |  12 +++
 9 files changed, 98 insertions(+), 94 deletions(-)
Range-diff against v3:
1:  4b320edc933 ! 1:  4dd317ab65e wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c
    @@ Commit message
         errno?) the worst we'll do is die with a nonsensical errno value, but
         we'll want to die in either case.
     
    -    We could make these return "void" (as far as I can tell there's no
    -    other x*() wrappers that needed to make that decision before),
    -    i.e. our "return 0" is only to indicate that we didn't error, which we
    -    would have died on. Let's return "int" instead to be consistent with
    -    the C library function signatures, including for any future code that
    -    expects a pointer to a setenv()-like function.
    +    Let's make these return "void" instead of "int". As far as I can tell
    +    there's no other x*() wrappers that needed to make the decision of
    +    deviating from the signature in the C library, but since their return
    +    value is only used to indicate errors (so we'd die here), we can catch
    +    unreachable code such as
    +
    +        if (xsetenv(...) < 0)
    +            [...];
     
         I think it would be OK skip the NULL check of the "name" here for the
         calls to die_errno(). Almost all of our setenv() callers are taking a
    @@ git-compat-util.h: void *xmemdupz(const void *data, size_t len);
      char *xstrndup(const char *str, size_t len);
      void *xrealloc(void *ptr, size_t size);
      void *xcalloc(size_t nmemb, size_t size);
    -+int xsetenv(const char *name, const char *value, int overwrite);
    -+int xunsetenv(const char *name);
    ++void xsetenv(const char *name, const char *value, int overwrite);
    ++void xunsetenv(const char *name);
      void *xmmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
      const char *mmap_os_err(void);
      void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);
    @@ wrapper.c: void *xcalloc(size_t nmemb, size_t size)
      	return ret;
      }
      
    -+int xsetenv(const char *name, const char *value, int overwrite)
    ++void xsetenv(const char *name, const char *value, int overwrite)
     +{
     +	if (setenv(name, value, overwrite))
    -+		die_errno("setenv(%s, '%s', %d) failed", name ? name : "(null)",
    -+			  value, overwrite);
    -+	return 0;
    ++		die_errno(_("could not setenv '%s'"), name ? name : "(null)");
     +}
     +
    -+int xunsetenv(const char *name)
    ++void xunsetenv(const char *name)
     +{
     +	if (!unsetenv(name))
    -+		die_errno("unsetenv(%s) failed", name ? name : "(null)");
    -+	return 0;
    ++		die_errno(_("could not unsetenv '%s'"), name ? name : "(null)");
     +}
     +
      /*
2:  ece340af764 = 2:  3dc37521184 environment.c: remove test-specific "ignore_untracked..." variable
3:  d837d905825 ! 3:  b36b23ee173 read-cache & fetch-negotiator: check "enum" values in switch()
    @@ fetch-negotiator.c: void fetch_negotiator_init(struct repository *r,
      		return;
     +	case FETCH_NEGOTIATION_NONE:
     +	case FETCH_NEGOTIATION_UNSET:
    -+		BUG("FETCH_NEGOTIATION_UNSET only in prepare_repo_settings()");
    ++		BUG("FETCH_NEGOTIATION_{NONE,UNSET} used outside of prepare_repo_settings()!");
      	}
      }
     
    @@ read-cache.c: static void tweak_untracked_cache(struct index_state *istate)
     +	case UNTRACKED_CACHE_KEEP:
     +		break;
     +	case UNTRACKED_CACHE_UNSET:
    -+		BUG("UNTRACKED_CACHE_UNSET only in prepare_repo_settings()");
    ++		BUG("UNTRACKED_CACHE_UNSET used outside of prepare_repo_settings()!");
     +	}
      }
      
4:  28286a61162 ! 4:  c9f143b26f1 repo-settings.c: simplify the setup
    @@ fetch-negotiator.c: void fetch_negotiator_init(struct repository *r,
      		return;
     -	case FETCH_NEGOTIATION_NONE:
     -	case FETCH_NEGOTIATION_UNSET:
    --		BUG("FETCH_NEGOTIATION_UNSET only in prepare_repo_settings()");
    +-		BUG("FETCH_NEGOTIATION_{NONE,UNSET} used outside of prepare_repo_settings()!");
      	}
      }
     
    @@ read-cache.c: static void tweak_untracked_cache(struct index_state *istate)
     +		 */
      		break;
     -	case UNTRACKED_CACHE_UNSET:
    --		BUG("UNTRACKED_CACHE_UNSET only in prepare_repo_settings()");
    +-		BUG("UNTRACKED_CACHE_UNSET used outside of prepare_repo_settings()!");
      	}
      }
      
5:  3cc033b8864 = 5:  aadd4c42923 repository.h: don't use a mix of int and bitfields
-- 
2.33.0.1098.gf02a64c1a2d
Previous: Taylor BlauNext: Ævar Arnfjörð Bjarmason
Message 26 of 34 in “repo-settings.c: refactor for clarity, get rid of hacks etc.”
  1. 0/5 repo-settings.c: refactor for clarity, get rid of hacks etc.Ævar Arnfjörð Bjarmason, Sep 16, 2021
  2. 1/5 wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.cÆvar Arnfjörð Bjarmason, Sep 16, 2021
  3. Junio C HamanoSep 17, 2021
  4. Ævar Arnfjörð BjarmasonSep 17, 2021
  5. 2/5 environment.c: remove test-specific "ignore_untracked..." variableÆvar Arnfjörð Bjarmason, Sep 16, 2021
  6. Junio C HamanoSep 17, 2021
  7. 3/5 read-cache & fetch-negotiator: check "enum" values in switch()Ævar Arnfjörð Bjarmason, Sep 16, 2021
  8. Junio C HamanoSep 17, 2021
  9. 5/5 repository.h: don't use a mix of int and bitfieldsÆvar Arnfjörð Bjarmason, Sep 16, 2021
  10. 4/5 repo-settings.c: simplify the setupÆvar Arnfjörð Bjarmason, Sep 16, 2021
  11. 0/5 repo-settings.c: refactor for clarity, get rid of hacks etc.Ævar Arnfjörð Bjarmason, Sep 19, 2021
  12. 2/5 environment.c: remove test-specific "ignore_untracked..." variableÆvar Arnfjörð Bjarmason, Sep 19, 2021
  13. Taylor BlauSep 20, 2021
  14. Ævar Arnfjörð BjarmasonSep 20, 2021
  15. 1/5 wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.cÆvar Arnfjörð Bjarmason, Sep 19, 2021
  16. Taylor BlauSep 20, 2021
  17. Ævar Arnfjörð BjarmasonSep 20, 2021
  18. 3/5 read-cache & fetch-negotiator: check "enum" values in switch()Ævar Arnfjörð Bjarmason, Sep 19, 2021
  19. Taylor BlauSep 20, 2021
  20. Ævar Arnfjörð BjarmasonSep 20, 2021
  21. 5/5 repository.h: don't use a mix of int and bitfieldsÆvar Arnfjörð Bjarmason, Sep 19, 2021
  22. Taylor BlauSep 20, 2021
  23. 4/5 repo-settings.c: simplify the setupÆvar Arnfjörð Bjarmason, Sep 19, 2021
  24. Derrick StoleeSep 20, 2021
  25. Taylor BlauSep 20, 2021
  26. 0/5 repo-settings.c: refactor for clarity, get rid of hacks etc.Ævar Arnfjörð Bjarmason, Sep 21, 2021
  27. 1/5 wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.cÆvar Arnfjörð Bjarmason, Sep 21, 2021
  28. 2/5 environment.c: remove test-specific "ignore_untracked..." variableÆvar Arnfjörð Bjarmason, Sep 21, 2021
  29. 3/5 read-cache & fetch-negotiator: check "enum" values in switch()Ævar Arnfjörð Bjarmason, Sep 21, 2021
  30. 4/5 repo-settings.c: simplify the setupÆvar Arnfjörð Bjarmason, Sep 21, 2021
  31. 5/5 repository.h: don't use a mix of int and bitfieldsÆvar Arnfjörð Bjarmason, Sep 21, 2021
  32. Derrick StoleeSep 21, 2021
  33. Taylor BlauSep 21, 2021
  34. Junio C HamanoSep 22, 2021

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.