Skip to content
Commits on Sep 25, 2015
  1. @peff @gitster

    replace trivial malloc + sprintf / strcpy calls with xstrfmt

    peff committed with gitster
    It's a common pattern to do:
    
      foo = xmalloc(strlen(one) + strlen(two) + 1 + 1);
      sprintf(foo, "%s %s", one, two);
    
    (or possibly some variant with strcpy()s or a more
    complicated length computation).  We can switch these to use
    xstrfmt, which is shorter, involves less error-prone manual
    computation, and removes many sprintf and strcpy calls which
    make it harder to audit the code for real buffer overflows.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Jun 25, 2015
  1. @peff @gitster

    introduce "preciousObjects" repository extension

    peff committed with gitster
    If this extension is used in a repository, then no
    operations should run which may drop objects from the object
    storage. This can be useful if you are sharing that storage
    with other repositories whose refs you cannot see.
    
    For instance, if you do:
    
      $ git clone -s parent child
      $ git -C parent config extensions.preciousObjects true
      $ git -C parent config core.repositoryformatversion 1
    
    you now have additional safety when running git in the
    parent repository. Prunes and repacks will bail with an
    error, and `git gc` will skip those operations (it will
    continue to pack refs and do other non-object operations).
    Older versions of git, when run in the repository, will
    fail on every operation.
    
    Note that we do not set the preciousObjects extension by
    default when doing a "clone -s", as doing so breaks
    backwards compatibility. It is a decision the user should
    make explicitly.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Mar 20, 2015
  1. @peff @gitster

    refs: introduce a "ref paranoia" flag

    peff committed with gitster
    Most operations that iterate over refs are happy to ignore
    broken cruft. However, some operations should be performed
    with knowledge of these broken refs, because it is better
    for the operation to choke on a missing object than it is to
    silently pretend that the ref did not exist (e.g., if we are
    computing the set of reachable tips in order to prune
    objects).
    
    These processes could just call for_each_rawref, except that
    ref iteration is often hidden behind other interfaces. For
    instance, for a destructive "repack -ad", we would have to
    inform "pack-objects" that we are destructive, and then it
    would in turn have to tell the revision code that our
    "--all" should include broken refs.
    
    It's much simpler to just set a global for "dangerous"
    operations that includes broken refs in all iterations.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Dec 17, 2014
  1. @peff @gitster

    read-cache: optionally disallow HFS+ .git variants

    peff committed with gitster
    The point of disallowing ".git" in the index is that we
    would never want to accidentally overwrite files in the
    repository directory. But this means we need to respect the
    filesystem's idea of when two paths are equal. The prior
    commit added a helper to make such a comparison for HFS+;
    let's use it in verify_path.
    
    We make this check optional for two reasons:
    
      1. It restricts the set of allowable filenames, which is
         unnecessary for people who are not on HFS+. In practice
         this probably doesn't matter, though, as the restricted
         names are rather obscure and almost certainly would
         never come up in practice.
    
      2. It has a minor performance penalty for every path we
         insert into the index.
    
    This patch ties the check to the core.protectHFS config
    option. Though this is expected to be most useful on OS X,
    we allow it to be set everywhere, as HFS+ may be mounted on
    other platforms. The variable does default to on for OS X,
    though.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Jun 25, 2014
  1. @peff @gitster

    setup_git_env(): introduce git_path_from_env() helper

    peff committed with gitster
    "Check the value of an environment and fall back to a known path
    inside $GIT_DIR" is repeated a few times to determine the location
    of the data store, the index and the graft file, but the return
    value of getenv is not guaranteed to survive across further
    invocations of setenv or even getenv.
    
    Make sure to xstrdup() the value we receive from getenv(3), and
    encapsulate the pattern into a helper function.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Jun 19, 2014
  1. @peff @gitster

    setup_git_env: use git_pathdup instead of xmalloc + sprintf

    peff committed with gitster
    This is shorter, harder to get wrong, and more clearly
    captures the intent.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Jul 12, 2013
  1. @peff @gitster

    cat-file: disable object/refname ambiguity check for batch mode

    peff committed with gitster
    A common use of "cat-file --batch-check" is to feed a list
    of objects from "rev-list --objects" or a similar command.
    In this instance, all of our input objects are 40-byte sha1
    ids. However, cat-file has always allowed arbitrary revision
    specifiers, and feeds the result to get_sha1().
    
    Fortunately, get_sha1() recognizes a 40-byte sha1 before
    doing any hard work trying to look up refs, meaning this
    scenario should end up spending very little time converting
    the input into an object sha1. However, since 798c35f
    (get_sha1: warn about full or short object names that look
    like refs, 2013-05-29), when we encounter this case, we
    spend the extra effort to do a refname lookup anyway, just
    to print a warning. This is further exacerbated by ca91993
    (get_packed_ref_cache: reload packed-refs file when it
    changes, 2013-06-20), which makes individual ref lookup more
    expensive by requiring a stat() of the packed-refs file for
    each missing ref.
    
    With no patches, this is the time it takes to run:
    
      $ git rev-list --objects --all >objects
      $ time git cat-file --batch-check='%(objectname)' <objects
    
    on the linux.git repository:
    
      real    1m13.494s
      user    0m25.924s
      sys     0m47.532s
    
    If we revert ca91993, the packed-refs up-to-date check, it
    gets a little better:
    
      real    0m54.697s
      user    0m21.692s
      sys     0m32.916s
    
    but we are still spending quite a bit of time on ref lookup
    (and we would not want to revert that patch, anyway, which
    has correctness issues).  If we revert 798c35f, disabling
    the warning entirely, we get a much more reasonable time:
    
      real    0m7.452s
      user    0m6.836s
      sys     0m0.608s
    
    This patch does the moral equivalent of this final case (and
    gets similar speedups). We introduce a global flag that
    callers of get_sha1() can use to avoid paying the price for
    the warning.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Mar 8, 2013
  1. @peff @gitster

    setup: suppress implicit "." work-tree for bare repos

    peff committed with gitster
    If an explicit GIT_DIR is given without a working tree, we
    implicitly assume that the current working directory should
    be used as the working tree. E.g.,:
    
      GIT_DIR=/some/repo.git git status
    
    would compare against the cwd.
    
    Unfortunately, we fool this rule for sub-invocations of git
    by setting GIT_DIR internally ourselves. For example:
    
      git init foo
      cd foo/.git
      git status ;# fails, as we expect
      git config alias.st status
      git status ;# does not fail, but should
    
    What happens is that we run setup_git_directory when doing
    alias lookup (since we need to see the config), set GIT_DIR
    as a result, and then leave GIT_WORK_TREE blank (because we
    do not have one). Then when we actually run the status
    command, we do setup_git_directory again, which sees our
    explicit GIT_DIR and uses the cwd as an implicit worktree.
    
    It's tempting to argue that we should be suppressing that
    second invocation of setup_git_directory, as it could use
    the values we already found in memory. However, the problem
    still exists for sub-processes (e.g., if "git status" were
    an external command).
    
    You can see another example with the "--bare" option, which
    sets GIT_DIR explicitly. For example:
    
      git init foo
      cd foo/.git
      git status ;# fails
      git --bare status ;# does NOT fail
    
    We need some way of telling sub-processes "even though
    GIT_DIR is set, do not use cwd as an implicit working tree".
    We could do it by putting a special token into
    GIT_WORK_TREE, but the obvious choice (an empty string) has
    some portability problems.
    
    Instead, we add a new boolean variable, GIT_IMPLICIT_WORK_TREE,
    which suppresses the use of cwd as a working tree when
    GIT_DIR is set. We trigger the new variable when we know we
    are in a bare setting.
    
    The variable is left intentionally undocumented, as this is
    an internal detail (for now, anyway). If somebody comes up
    with a good alternate use for it, and once we are confident
    we have shaken any bugs out of it, we can consider promoting
    it further.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  2. @peff @gitster

    environment: add GIT_PREFIX to local_repo_env

    peff committed with gitster
    The GIT_PREFIX variable is set based on our location within
    the working tree. It should therefore be cleared whenever
    GIT_WORK_TREE is cleared.
    
    In practice, this doesn't cause any bugs, because none of
    the sub-programs we invoke with local_repo_env cleared
    actually care about GIT_PREFIX. But this is the right thing
    to do, and future proofs us against that assumption changing.
    
    While we're at it, let's define a GIT_PREFIX_ENVIRONMENT
    macro; this avoids repetition of the string literal, which
    can help catch any spelling mistakes in the code.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
  3. @peff @gitster

    cache.h: drop LOCAL_REPO_ENV_SIZE

    peff committed with gitster
    We keep a static array of variables that should be cleared
    when invoking a sub-process on another repo. We statically
    size the array with the LOCAL_REPO_ENV_SIZE macro so that
    any readers do not have to count it themselves.
    
    As it turns out, no readers actually use the macro, and it
    creates a maintenance headache, as modifications to the
    array need to happen in two places (one to add the new
    element, and another to bump the size).
    
    Since it's NULL-terminated, we can just drop the size macro
    entirely. While we're at it, we'll clean up some comments
    around it, and add a new mention of it at the top of the
    list of environment variable macros. Even though
    local_repo_env is right below that list, it's easy to miss,
    and additions to that list should consider local_repo_env.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on May 22, 2012
  1. @peff @gitster

    move git_default_* variables to ident.c

    peff committed with gitster
    There's no reason anybody outside of ident.c should access
    these directly (they should use the new accessors which make
    sure the variables are initialized), so we can make them
    file-scope statics.
    
    While we're at it, move user_ident_explicitly_given into
    ident.c; while still globally visible, it makes more sense
    to reside with the ident code.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commits on Dec 11, 2007
  1. @peff @gitster

    Support GIT_PAGER_IN_USE environment variable

    peff committed with gitster
    When deciding whether or not to turn on automatic color
    support, git_config_colorbool checks whether stdout is a
    tty. However, because we run a pager, if stdout is not a
    tty, we must check whether it is because we started the
    pager. This used to be done by checking the pager_in_use
    variable.
    
    This variable was set only when the git program being run
    started the pager; there was no way for an external program
    running git indicate that it had already started a pager.
    This patch allows a program to set GIT_PAGER_IN_USE to a
    true value to indicate that even though stdout is not a tty,
    it is because a pager is being used.
    
    Signed-off-by: Jeff King <peff@peff.net>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
Something went wrong with that request. Please try again.