1. 22 Feb, 2016 3 commits
    • Jeff King's avatar
      use xmallocz to avoid size arithmetic · 3733e694
      Jeff King authored
      We frequently allocate strings as xmalloc(len + 1), where
      the extra 1 is for the NUL terminator. This can be done more
      simply with xmallocz, which also checks for integer
      overflow.
      
      There's no case where switching xmalloc(n+1) to xmallocz(n)
      is wrong; the result is the same length, and malloc made no
      guarantees about what was in the buffer anyway. But in some
      cases, we can stop manually placing NUL at the end of the
      allocated buffer. But that's only safe if it's clear that
      the contents will always fill the buffer.
      
      In each case where this patch does so, I manually examined
      the control flow, and I tried to err on the side of caution.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      3733e694
    • Patrick Steinhardt's avatar
      config: rename git_config_set_or_die to git_config_set · 3d180648
      Patrick Steinhardt authored
      Rename git_config_set_or_die functions to git_config_set, leading
      to the new default behavior of dying whenever a configuration
      error occurs.
      
      By now all callers that shall die on error have been transitioned
      to the _or_die variants, thus making this patch a simple rename
      of the functions.
      Signed-off-by: default avatarPatrick Steinhardt <ps@pks.im>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      3d180648
    • Patrick Steinhardt's avatar
      config: rename git_config_set to git_config_set_gently · 30598ad0
      Patrick Steinhardt authored
      The desired default behavior for `git_config_set` is to die
      whenever an error occurs. Dying is the default for a lot of
      internal functions when failures occur and is in this case the
      right thing to do for most callers as otherwise we might run into
      inconsistent repositories without noticing.
      
      As some code may rely on the actual return values for
      `git_config_set` we still require the ability to invoke these
      functions without aborting. Rename the existing `git_config_set`
      functions to `git_config_set_gently` to keep them available for
      those callers.
      Signed-off-by: default avatarPatrick Steinhardt <ps@pks.im>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      30598ad0
  2. 16 Feb, 2016 1 commit
    • Patrick Steinhardt's avatar
      config: introduce set_or_die wrappers · b4c8aba6
      Patrick Steinhardt authored
      A lot of call-sites for the existing family of `git_config_set`
      functions do not check for errors that may occur, e.g. when the
      configuration file is locked. In many cases we simply want to die
      when such a situation arises.
      
      Introduce wrappers that will cause the program to die in those
      cases. These wrappers are temporary only to ease the transition
      to let `git_config_set` die by default. They will be removed
      later on when `git_config_set` itself has been replaced by
      `git_config_set_gently`.
      Signed-off-by: default avatarPatrick Steinhardt <ps@pks.im>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      b4c8aba6
  3. 01 Dec, 2015 1 commit
    • SZEDER Gábor's avatar
      Make error message after failing commit_lock_file() less confusing · 08a3651f
      SZEDER Gábor authored
      The error message after a failing commit_lock_file() call sometimes
      looks like this, causing confusion:
      
        $ git remote add remote git@server.com/repo.git
        error: could not commit config file .git/config
        # Huh?!
        # I didn't want to commit anything, especially not my config file!
      
      While in the narrow context of the lockfile module using the verb
      'commit' in the error message makes perfect sense, in the broader
      context of git the word 'commit' already has a very specific meaning,
      hence the confusion.
      
      Reword these error messages to say "could not write" instead of "could
      not commit".
      
      While at it, include strerror in the error messages after writing the
      config file or the credential store fails to provide some information
      about the cause of the failure, and update the style of the error
      message after writing the reflog fails to match surrounding error
      messages (i.e. no '' around the pathname and no () around the error
      description).
      Signed-off-by: default avatarSZEDER Gábor <szeder@ira.uka.de>
      Signed-off-by: default avatarJeff King <peff@peff.net>
      08a3651f
  4. 24 Aug, 2015 1 commit
    • Jeff King's avatar
      config: silence warnings for command names with invalid keys · 9e9de18f
      Jeff King authored
      When we are running the git command "foo", we may have to
      look up the config keys "pager.foo" and "alias.foo". These
      config schemes are mis-designed, as the command names can be
      anything, but the config syntax has some restrictions. For
      example:
      
        $ git foo_bar
        error: invalid key: pager.foo_bar
        error: invalid key: alias.foo_bar
        git: 'foo_bar' is not a git command. See 'git --help'.
      
      You cannot name an alias with an underscore. And if you have
      an external command with one, you cannot configure its
      pager.
      
      In the long run, we may develop a different config scheme
      for these features. But in the near term (and because we'll
      need to support the existing scheme indefinitely), we should
      at least squelch the error messages shown above.
      
      These errors come from git_config_parse_key. Ideally we
      would pass a "quiet" flag to the config machinery, but there
      are many layers between the pager code and the key parsing.
      Passing a flag through all of those would be an invasive
      change.
      
      Instead, let's provide a config function to report on
      whether a key is syntactically valid, and have the pager and
      alias code skip lookup for bogus keys. We can build this
      easily around the existing git_config_parse_key, with two
      minor modifications:
      
        1. We now handle a NULL store_key, to validate but not
           write out the normalized key.
      
        2. We accept a "quiet" flag to avoid writing to stderr.
           This doesn't need to be a full-blown public "flags"
           field, because we can make the existing implementation
           a static helper function, keeping the mess contained
           inside config.c.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      9e9de18f
  5. 19 Aug, 2015 1 commit
  6. 14 Aug, 2015 1 commit
  7. 10 Aug, 2015 1 commit
  8. 30 Jun, 2015 1 commit
  9. 28 May, 2015 3 commits
    • Jeff King's avatar
      config.c: rewrite ENODEV into EISDIR when mmap fails · 0e8771f1
      Jeff King authored
      If we try to mmap a directory, we'll get ENODEV. This
      translates to "no such device" for the user, which is not
      very helpful. Since we've just fstat()'d the file, we can
      easily check whether the problem was a directory to give a
      better message.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      0e8771f1
    • Jeff King's avatar
      config.c: avoid xmmap error messages · 1570856b
      Jeff King authored
      The config-writing code uses xmmap to map the existing
      config file, which will die if the map fails. This has two
      downsides:
      
        1. The error message is not very helpful, as it lacks any
           context about the file we are mapping:
      
             $ mkdir foo
             $ git config --file=foo some.key value
             fatal: Out of memory? mmap failed: No such device
      
        2. We normally do not die in this code path; instead, we'd
           rather report the error and return an appropriate exit
           status (which is part of the public interface
           documented in git-config.1).
      
      This patch introduces a "gentle" form of xmmap which lets us
      produce our own error message. We do not want to use mmap
      directly, because we would like to use the other
      compatibility elements of xmmap (e.g., handling 0-length
      maps portably).
      
      The end result is:
      
          $ git.compile config --file=foo some.key value
          error: unable to mmap 'foo': No such device
          $ echo $?
          3
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      1570856b
    • Jeff King's avatar
      config.c: fix mmap leak when writing config · 3a1b3126
      Jeff King authored
      We mmap the existing config file, but fail to unmap it if we
      hit an error. The function already has a shared exit path,
      so we can fix this by moving the mmap pointer to the
      function scope and clearing it in the shared exit.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      3a1b3126
  10. 06 May, 2015 1 commit
  11. 16 Apr, 2015 2 commits
    • Junio C Hamano's avatar
      config: use utf8_bom[] from utf.[ch] in git_parse_source() · 599446dc
      Junio C Hamano authored
      Because the function reads one character at the time, unfortunately
      we cannot use the easier skip_utf8_bom() helper, but at least we do
      not have to duplicate the constant string this way.
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      599446dc
    • Jeff King's avatar
      config: use getc_unlocked when reading from file · 260d408e
      Jeff King authored
      We read config files character-by-character from a stdio
      handle using fgetc(). This incurs significant locking
      overhead, even though we know that only one thread can
      possibly access the handle. We can speed this up by taking
      the lock ourselves, and then using getc_unlocked to read
      each character.
      
      On a silly pathological case:
      
        perl -le '
          print "[core]";
          print "key$_ = value$_" for (1..1000000)
        ' >input
        git config -f input core.key1
      
      this dropped the time to run git-config from:
      
        real    0m0.263s
        user    0m0.260s
        sys     0m0.000s
      
      to:
      
        real    0m0.159s
        user    0m0.152s
        sys     0m0.004s
      
      for a savings of 39%.  Most config files are not this big,
      but the savings should be proportional to the size of the
      file (i.e., we always save 39%, just of a much smaller
      number).
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      260d408e
  12. 05 Feb, 2015 2 commits
    • Jeff King's avatar
      config_buf_ungetc: warn when pushing back a random character · 1d0655c1
      Jeff King authored
      Our config code simulates a stdio stream around a buffer,
      but our fake ungetc() does not behave quite like the real
      one. In particular, we only rewind the position by one
      character, but do _not_ actually put the character from the
      caller into position.
      
      It turns out that this does not matter, because we only ever
      push back the character we just read. In other words, such
      an assignment would be a noop. But because the function is
      called ungetc, and because it takes a character parameter,
      it is a mistake waiting to happen.
      
      Actually assigning the character into the buffer would be
      ideal, but our pointer is actually a "const" copy of the
      buffer. We do not know who the real owner of the buffer is
      in this code, and would not want to munge their contents.
      
      Instead, we can simply add an assertion that matches what
      the current caller does, and will let us know if new callers
      are added that violate the contract.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      1d0655c1
    • Jeff King's avatar
      config: do not ungetc EOF · 5e0be134
      Jeff King authored
      When we are parsing a config value, if we see a carriage
      return, we fgetc the next character to see if it is a
      line feed (in which case we silently drop the CR). If it
      isn't, we then ungetc the character, and take the literal
      CR.
      
      But we never check whether we in fact got a character at
      all. If the config file ends in CR, we will get EOF here,
      and try to ungetc EOF. This works OK for a real stdio
      stream. The ungetc returns an error, and the next fgetc will
      then return EOF again.
      
      However, our custom buffer-based stream is not so fortunate.
      It happily rewinds the position of the stream by one
      character, ignoring the fact that we fed it EOF. The next
      fgetc call returns the final CR again, over and over, and we
      end up in an infinite loop.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      5e0be134
  13. 13 Jan, 2015 1 commit
  14. 17 Dec, 2014 2 commits
    • Johannes Schindelin's avatar
      read-cache: optionally disallow NTFS .git variants · 2b4c6efc
      Johannes Schindelin authored
      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 NTFS
      and FAT32; 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 NTFS nor FAT32.
           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.protectNTFS config
      option. Though this is expected to be most useful on Windows,
      we allow it to be set everywhere, as NTFS may be mounted on
      other platforms. The variable does default to on for Windows,
      though.
      Signed-off-by: Johannes Schindelin's avatarJohannes Schindelin <johannes.schindelin@gmx.de>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      2b4c6efc
    • Jeff King's avatar
      read-cache: optionally disallow HFS+ .git variants · a42643aa
      Jeff King authored
      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: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      a42643aa
  15. 17 Nov, 2014 1 commit
  16. 01 Oct, 2014 3 commits
  17. 11 Sep, 2014 1 commit
  18. 02 Sep, 2014 1 commit
  19. 28 Aug, 2014 1 commit
  20. 18 Aug, 2014 1 commit
    • Tanay Abhra's avatar
      make config --add behave correctly for empty and NULL values · c8466645
      Tanay Abhra authored
      Currently if we have a config file like,
      [foo]
              baz
              bar =
      
      and we try something like, "git config --add foo.baz roll", Git will
      segfault. Moreover, for "git config --add foo.bar roll", it will
      overwrite the original value instead of appending after the existing
      empty value.
      
      The problem lies with the regexp used for simulating --add in
      `git_config_set_multivar_in_file()`, "^$", which in ideal case should
      not match with any string but is true for empty strings. Instead use a
      regexp like "a^" which can not be true for any string, empty or not.
      
      For removing the segfault add a check for NULL values in `matches()` in
      config.c.
      Signed-off-by: default avatarTanay Abhra <tanayabh@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      c8466645
  21. 07 Aug, 2014 6 commits
  22. 05 Aug, 2014 1 commit
  23. 29 Jul, 2014 1 commit
    • Tanay Abhra's avatar
      add `config_set` API for caching config-like files · 3c8687a7
      Tanay Abhra authored
      Currently `git_config()` uses a callback mechanism and file rereads for
      config values. Due to this approach, it is not uncommon for the config
      files to be parsed several times during the run of a git program, with
      different callbacks picking out different variables useful to themselves.
      
      Add a `config_set`, that can be used to construct an in-memory cache for
      config-like files that the caller specifies (i.e., files like `.gitmodules`,
      `~/.gitconfig` etc.). Add two external functions `git_configset_get_value`
      and `git_configset_get_value_multi` for querying from the config sets.
      `git_configset_get_value` follows `last one wins` semantic (i.e. if there
      are multiple matches for the queried key in the files of the configset the
      value returned will be the last entry in `value_list`).
      `git_configset_get_value_multi` returns a list of values sorted in order of
      increasing priority (i.e. last match will be at the end of the list). Add
      type specific query functions like `git_configset_get_bool` and similar.
      
      Add a default `config_set`, `the_config_set` to cache all key-value pairs
      read from usual config files (repo specific .git/config, user wide
      ~/.gitconfig, XDG config and the global /etc/gitconfig). `the_config_set`
      is populated using `git_config()`.
      
      Add two external functions `git_config_get_value` and
      `git_config_get_value_multi` for querying in a non-callback manner from
      `the_config_set`. Also, add type specific query functions that are
      implemented as a thin wrapper around the `config_set` API.
      Signed-off-by: default avatarMatthieu Moy <Matthieu.Moy@imag.fr>
      Signed-off-by: default avatarTanay Abhra <tanayabh@gmail.com>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      3c8687a7
  24. 24 Jul, 2014 1 commit
    • Jeff King's avatar
      fix memory leak parsing core.commentchar · 649409b7
      Jeff King authored
      When we see the core.commentchar config option, we extract
      the string with git_config_string, which does two things:
      
        1. It complains via config_error_nonbool if there is no
           string value.
      
        2. It makes a copy of the string.
      
      Since we immediately parse the string into its
      single-character value, we only care about (1). And in fact
      (2) is a detriment, as it means we leak the copy. Instead,
      let's just check the pointer value ourselves, and parse
      directly from the const string we already have.
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      649409b7
  25. 16 Jul, 2014 1 commit
  26. 20 Jun, 2014 1 commit
    • Jeff King's avatar
      refactor skip_prefix to return a boolean · cf4fff57
      Jeff King authored
      The skip_prefix() function returns a pointer to the content
      past the prefix, or NULL if the prefix was not found. While
      this is nice and simple, in practice it makes it hard to use
      for two reasons:
      
        1. When you want to conditionally skip or keep the string
           as-is, you have to introduce a temporary variable.
           For example:
      
             tmp = skip_prefix(buf, "foo");
             if (tmp)
      	       buf = tmp;
      
        2. It is verbose to check the outcome in a conditional, as
           you need extra parentheses to silence compiler
           warnings. For example:
      
             if ((cp = skip_prefix(buf, "foo"))
      	       /* do something with cp */
      
      Both of these make it harder to use for long if-chains, and
      we tend to use starts_with() instead. However, the first line
      of "do something" is often to then skip forward in buf past
      the prefix, either using a magic constant or with an extra
      strlen(3) (which is generally computed at compile time, but
      means we are repeating ourselves).
      
      This patch refactors skip_prefix() to return a simple boolean,
      and to provide the pointer value as an out-parameter. If the
      prefix is not found, the out-parameter is untouched. This
      lets you write:
      
        if (skip_prefix(arg, "foo ", &arg))
      	  do_foo(arg);
        else if (skip_prefix(arg, "bar ", &arg))
      	  do_bar(arg);
      Signed-off-by: default avatarJeff King <peff@peff.net>
      Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
      cf4fff57