1. 12 11月, 2017 1 次提交
    • R
      apply: avoid out-of-bounds access in fuzzy_matchlines() · 6ce15ce5
      René Scharfe 提交于
      fuzzy_matchlines() uses a pointers to the first and last characters of
      two lines to keep track while matching them.  This makes it impossible
      to deal with empty strings.  It accesses characters before the start of
      empty lines.  It can also access characters after the end when checking
      for trailing whitespace in the main loop.
      
      Avoid that by using pointers to the first character and the one *after*
      the last one.  This is well-defined as long as the latter is not
      dereferenced.  Basically rewrite the function based on that premise; it
      becomes much simpler as a result.  There is no need to check for
      leading whitespace outside of the main loop anymore.
      Reported-by: NMahmoud Al-Qudsi <mqudsi@neosmart.net>
      Signed-off-by: NRene Scharfe <l.s.r@web.de>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      6ce15ce5
  2. 22 9月, 2017 1 次提交
    • J
      consistently use "fallthrough" comments in switches · 1cf01a34
      Jeff King 提交于
      Gcc 7 adds -Wimplicit-fallthrough, which can warn when a
      switch case falls through to the next case. The general idea
      is that the compiler can't tell if this was intentional or
      not, so you should annotate any intentional fall-throughs as
      such, leaving it to complain about any unannotated ones.
      
      There's a GNU __attribute__ which can be used for
      annotation, but of course we'd have to #ifdef it away on
      non-gcc compilers. Gcc will also recognize
      specially-formatted comments, which matches our current
      practice. Let's extend that practice to all of the
      unannotated sites (which I did look over and verify that
      they were behaving as intended).
      
      Ideally in each case we'd actually give some reasons in the
      comment about why we're falling through, or what we're
      falling through to. And gcc does support that with
      -Wimplicit-fallthrough=2, which relaxes the comment pattern
      matching to anything that contains "fallthrough" (or a
      variety of spelling variants). However, this isn't the
      default for -Wimplicit-fallthrough, nor for -Wextra. In the
      name of simplicity, it's probably better for us to support
      the default level, which requires "fallthrough" to be the
      only thing in the comment (modulo some window dressing like
      "else" and some punctuation; see the gcc manual for the
      complete set of patterns).
      
      This patch suppresses all warnings due to
      -Wimplicit-fallthrough. We might eventually want to add that
      to the DEVELOPER Makefile knob, but we should probably wait
      until gcc 7 is more widely adopted (since earlier versions
      will complain about the unknown warning type).
      Signed-off-by: NJeff King <peff@peff.net>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      1cf01a34
  3. 26 8月, 2017 2 次提交
  4. 20 8月, 2017 1 次提交
    • T
      apply: file commited with CRLF should roundtrip diff and apply · c24f3aba
      Torsten Bögershausen 提交于
      When a file had been commited with CRLF but now .gitattributes say
      "* text=auto" (or core.autocrlf is true), the following does not
      roundtrip, `git apply` fails:
      
          printf "Added line\r\n" >>file &&
          git diff >patch &&
          git checkout -- . &&
          git apply patch
      
      Before applying the patch, the file from working tree is converted
      into the index format (clean filter, CRLF conversion, ...).  Here,
      when commited with CRLF, the line endings should not be converted.
      
      Note that `git apply --index` or `git apply --cache` doesn't call
      convert_to_git() because the source material is already in index
      format.
      
      Analyze the patch if there is a) any context line with CRLF, or b)
      if any line with CRLF is to be removed.  In this case the patch file
      `patch` has mixed line endings, for a) it looks like this:
      
          diff --git a/one b/one
          index 533790e..c30dea8 100644
          --- a/one
          +++ b/one
          @@ -1 +1,2 @@
           a\r
          +b\r
      
      And for b) it looks like this:
      
          diff --git a/one b/one
          index 533790e..485540d 100644
          --- a/one
          +++ b/one
          @@ -1 +1 @@
          -a\r
          +b\r
      
      If `git apply` detects that the patch itself has CRLF, (look at the
      line " a\r" or "-a\r" above), the new flag crlf_in_old is set in
      "struct patch" and two things will happen:
      
          - read_old_data() will not convert CRLF into LF by calling
            convert_to_git(..., SAFE_CRLF_KEEP_CRLF);
          - The WS_CR_AT_EOL bit is set in the "white space rule",
            CRLF are no longer treated as white space.
      
      While at there, make it clear that read_old_data() in apply.c knows
      what it wants convert_to_git() to do with respect to CRLF.  In fact,
      this codepath is about applying a patch to a file in the filesystem,
      which may not exist in the index, or may exist but may not match
      what is recorded in the index, or in the extreme case, we may not
      even be in a Git repository.  If convert_to_git() peeked at the
      index while doing its work, it *would* be a bug.
      
      Pass NULL instead of &the_index to convert_to_git() to make sure we
      catch future bugs to clarify this.
      
      Update the test in t4124: split one test case into 3:
      
          - Detect the " a\r" line in the patch
          - Detect the "-a\r" line in the patch
          - Use LF in repo and CLRF in the worktree.
      Reported-by: NAnthony Sottile <asottile@umich.edu>
      Signed-off-by: NTorsten Bögershausen <tboegi@web.de>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      c24f3aba
  5. 10 8月, 2017 1 次提交
  6. 18 7月, 2017 2 次提交
    • R
      apply: use COPY_ARRAY and MOVE_ARRAY in update_image() · 17736641
      René Scharfe 提交于
      Simplify the code by using the helper macros COPY_ARRAY and MOVE_ARRAY,
      which also makes them more robust in the case we copy or move no lines,
      as they allow using NULL points in that case, while memcpy(3) and
      memmove(3) don't.
      
      Found with Clang's UBSan.
      Signed-off-by: NRene Scharfe <l.s.r@web.de>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      17736641
    • B
      sha1_name: convert get_sha1* to get_oid* · e82caf38
      brian m. carlson 提交于
      Now that all the callers of get_sha1 directly or indirectly use struct
      object_id, rename the functions starting with get_sha1 to start with
      get_oid.  Convert the internals in sha1_name.c to use struct object_id
      as well, and eliminate explicit length checks where possible.  Convert a
      use of 40 in get_oid_basic to GIT_SHA1_HEXSZ.
      
      Outside of sha1_name.c and cache.h, this transition was made with the
      following semantic patch:
      
      @@
      expression E1, E2;
      @@
      - get_sha1(E1, E2.hash)
      + get_oid(E1, &E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1(E1, E2->hash)
      + get_oid(E1, E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_committish(E1, E2.hash)
      + get_oid_committish(E1, &E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_committish(E1, E2->hash)
      + get_oid_committish(E1, E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_treeish(E1, E2.hash)
      + get_oid_treeish(E1, &E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_treeish(E1, E2->hash)
      + get_oid_treeish(E1, E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_commit(E1, E2.hash)
      + get_oid_commit(E1, &E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_commit(E1, E2->hash)
      + get_oid_commit(E1, E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_tree(E1, E2.hash)
      + get_oid_tree(E1, &E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_tree(E1, E2->hash)
      + get_oid_tree(E1, E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_blob(E1, E2.hash)
      + get_oid_blob(E1, &E2)
      
      @@
      expression E1, E2;
      @@
      - get_sha1_blob(E1, E2->hash)
      + get_oid_blob(E1, E2)
      
      @@
      expression E1, E2, E3, E4;
      @@
      - get_sha1_with_context(E1, E2, E3.hash, E4)
      + get_oid_with_context(E1, E2, &E3, E4)
      
      @@
      expression E1, E2, E3, E4;
      @@
      - get_sha1_with_context(E1, E2, E3->hash, E4)
      + get_oid_with_context(E1, E2, E3, E4)
      Signed-off-by: Nbrian m. carlson <sandals@crustytoothpaste.net>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      e82caf38
  7. 10 7月, 2017 1 次提交
  8. 02 7月, 2017 1 次提交
  9. 28 6月, 2017 3 次提交
  10. 24 6月, 2017 1 次提交
  11. 17 6月, 2017 1 次提交
  12. 16 6月, 2017 1 次提交
  13. 14 6月, 2017 1 次提交
  14. 13 6月, 2017 1 次提交
  15. 30 5月, 2017 1 次提交
  16. 09 5月, 2017 1 次提交
    • J
      apply.c: fix whitespace-only mismerge · e294e895
      Junio C Hamano 提交于
      4af9a7d3 ("Merge branch 'bc/object-id'", 2016-09-19) involved
      merging a lot of changes made to builtin/apply.c on the side branch
      manually to apply.c as an intervening commit 13b5af22 ("apply: move
      libified code from builtin/apply.c to apply.{c,h}", 2016-04-22)
      moved a lot of the lines changed on the side branch to a different
      file apply.c at the top-level, requiring manual patching of it.
      Apparently, the maintainer screwed up and made the code indent in a
      funny way while doing so.
      Reported-by: NStefan Beller <sbeller@google.com>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      e294e895
  17. 22 3月, 2017 2 次提交
    • J
      prefix_filename: return newly allocated string · e4da43b1
      Jeff King 提交于
      The prefix_filename() function returns a pointer to static
      storage, which makes it easy to use dangerously. We already
      fixed one buggy caller in hash-object recently, and the
      calls in apply.c are suspicious (I didn't dig in enough to
      confirm that there is a bug, but we call the function once
      in apply_all_patches() and then again indirectly from
      parse_chunk()).
      
      Let's make it harder to get wrong by allocating the return
      value. For simplicity, we'll do this even when the prefix is
      empty (and we could just return the original file pointer).
      That will cause us to allocate sometimes when we wouldn't
      otherwise need to, but this function isn't called in
      performance critical code-paths (and it already _might_
      allocate on any given call, so a caller that cares about
      performance is questionable anyway).
      
      The downside is that the callers need to remember to free()
      the result to avoid leaking. Most of them already used
      xstrdup() on the result, so we know they are OK. The
      remainder have been converted to use free() as appropriate.
      
      I considered retaining a prefix_filename_unsafe() for cases
      where we know the static lifetime is OK (and handling the
      cleanup is awkward). This is only a handful of cases,
      though, and it's not worth the mental energy in worrying
      about whether the "unsafe" variant is OK to use in any
      situation.
      Signed-off-by: NJeff King <peff@peff.net>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      e4da43b1
    • J
      prefix_filename: drop length parameter · 116fb64e
      Jeff King 提交于
      This function takes the prefix as a ptr/len pair, but in
      every caller the length is exactly strlen(ptr). Let's
      simplify the interface and just take the string. This saves
      callers specifying it (and in some cases handling a NULL
      prefix).
      
      In a handful of cases we had the length already without
      calling strlen, so this is technically slower. But it's not
      likely to matter (after all, if the prefix is non-empty
      we'll allocate and copy it into a buffer anyway).
      Signed-off-by: NJeff King <peff@peff.net>
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      116fb64e
  18. 31 1月, 2017 1 次提交
  19. 08 12月, 2016 1 次提交
    • J
      hold_locked_index(): align error handling with hold_lockfile_for_update() · b3e83cc7
      Junio C Hamano 提交于
      Callers of the hold_locked_index() function pass 0 when they want to
      prepare to write a new version of the index file without wishing to
      die or emit an error message when the request fails (e.g. somebody
      else already held the lock), and pass 1 when they want the call to
      die upon failure.
      
      This option is called LOCK_DIE_ON_ERROR by the underlying lockfile
      API, and the hold_locked_index() function translates the paramter to
      LOCK_DIE_ON_ERROR when calling the hold_lock_file_for_update().
      
      Replace these hardcoded '1' with LOCK_DIE_ON_ERROR and stop
      translating.  Callers other than the ones that are replaced with
      this change pass '0' to the function; no behaviour change is
      intended with this patch.
      Signed-off-by: NJunio C Hamano <gitster@pobox.com>
      ---
      
      Among the callers of hold_locked_index() that passes 0:
      
       - diff.c::refresh_index_quietly() at the end of "git diff" is an
         opportunistic update; it leaks the lockfile structure but it is
         just before the program exits and nobody should care.
      
       - builtin/describe.c::cmd_describe(),
         builtin/commit.c::cmd_status(),
         sequencer.c::read_and_refresh_cache() are all opportunistic
         updates and they are OK.
      
       - builtin/update-index.c::cmd_update_index() takes a lock upfront
         but we may end up not needing to update the index (i.e. the
         entries may be fully up-to-date), in which case we do not need to
         issue an error upon failure to acquire the lock.  We do diagnose
         and die if we indeed need to update, so it is OK.
      
       - wt-status.c::require_clean_work_tree() IS BUGGY.  It asks
         silence, does not check the returned value.  Compare with
         callsites like cmd_describe() and cmd_status() to notice that it
         is wrong to call update_index_if_able() unconditionally.
      b3e83cc7
  20. 18 10月, 2016 1 次提交
  21. 15 10月, 2016 3 次提交
  22. 23 9月, 2016 1 次提交
  23. 08 9月, 2016 9 次提交
  24. 12 8月, 2016 2 次提交