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

Re: [PATCH v3 13/22] resolve_ref(): turn buffer into a proper string as soon as possible

From
Thomas Rast <trast@student.ethz.ch>
Date
Sep 23, 2011, 08:17 UTC
Message-ID
<201109231017.55996.trast@student.ethz.ch>
In-Reply-To
<1316121043-29367-14-git-send-email-mhagger@alum.mit.edu>
Hi Michael
Michael Haggerty wrote:
Show 7 quoted lines
> Immediately strip off trailing spaces and null-terminate the string
> holding the contents of the reference file; this allows the use of
> string functions and avoids the need to keep separate track of the
> string's length.  (get_sha1_hex() fails automatically if the string is
> too short.)
> 
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>

I'm getting valgrind failures in t1450-fsck and t3800-mktag which blame to this commit. For t1450 it looks as follows:

    ok 5 - object with bad sha1
    expecting success: 
            git rev-parse HEAD^{tree} >.git/refs/heads/invalid &&
            test_when_finished "git update-ref -d refs/heads/invalid" &&
            git fsck 2>out &&
            cat out &&
            grep "not a commit" out
    ==19623== Use of uninitialised value of size 8
    ==19623==    at 0x4B6747: hexval (cache.h:798)
    ==19623==    by 0x4B6797: get_sha1_hex (hex.c:42)
    ==19623==    by 0x4DD12A: resolve_ref (refs.c:588)
    ==19623==    by 0x4DC777: get_ref_dir (refs.c:313)
    ==19623==    by 0x4DC6FA: get_ref_dir (refs.c:302)
    ==19623==    by 0x4DC963: get_loose_refs (refs.c:368)
    ==19623==    by 0x4DD556: do_for_each_ref (refs.c:687)
    ==19623==    by 0x4DDA05: for_each_replace_ref (refs.c:806)
    ==19623==    by 0x4E5CE9: prepare_replace_object (replace_object.c:86)
    ==19623==    by 0x4E5D3C: do_lookup_replace_object (replace_object.c:103)
    ==19623==    by 0x4C99BB: lookup_replace_object (cache.h:764)
    ==19623==    by 0x4C9FA6: parse_object (object.c:191)
    ==19623==  Uninitialised value was created by a stack allocation
    ==19623==    at 0x4DCE34: resolve_ref (refs.c:498)

or when I run it at the tip of pu instead of at the commit itself, line numbers are like so:

    ==2308== Use of uninitialised value of size 8
    ==2308==    at 0x4ADB8B: get_sha1_hex (cache.h:800)
    ==2308==    by 0x4D4283: resolve_ref (refs.c:629)
    ==2308==    by 0x4D4851: get_ref_dir (refs.c:361)
    ==2308==    by 0x4D48C6: get_ref_dir (refs.c:350)
    ==2308==    by 0x4D4D29: do_for_each_ref (refs.c:412)
    ==2308==    by 0x4DCD93: do_lookup_replace_object (replace_object.c:86)
    ==2308==    by 0x4C31F4: parse_object (cache.h:764)
    ==2308==    by 0x4F2A1D: get_sha1_1 (sha1_name.c:567)
    ==2308==    by 0x4F2D5F: get_sha1_with_context_1 (sha1_name.c:1117)
    ==2308==    by 0x4F3543: get_sha1 (cache.h:822)
    ==2308==    by 0x461B50: cmd_rev_parse (rev-parse.c:723)
    ==2308==    by 0x404B71: run_builtin (git.c:308)
    ==2308==  Uninitialised value was created by a stack allocation
    ==2308==    at 0x4D4006: resolve_ref (refs.c:530)
Can you look into this?
-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Previous: Michael HaggertyNext: Michael Haggerty
Message 15 of 60 in “Clean up refname checks and normalization”
  1. 00/22 Clean up refname checks and normalizationMichael Haggerty, Sep 15, 2011
  2. 01/22 t1402: add some more testsMichael Haggerty, Sep 15, 2011
  3. 02/22 git check-ref-format: add options --allow-onelevel and --refspec-patternMichael Haggerty, Sep 15, 2011
  4. 03/22 Change bad_ref_char() to return a boolean valueMichael Haggerty, Sep 15, 2011
  5. 04/22 Change check_ref_format() to take a flags argumentMichael Haggerty, Sep 15, 2011
  6. 05/22 Refactor check_refname_format()Michael Haggerty, Sep 15, 2011
  7. 06/22 Do not allow ".lock" at the end of any refname componentMichael Haggerty, Sep 15, 2011
  8. 07/22 Make collapse_slashes() allocate memory for its resultMichael Haggerty, Sep 15, 2011
  9. 08/22 Inline function refname_format_print()Michael Haggerty, Sep 15, 2011
  10. 09/22 Change check_refname_format() to reject unnormalized refnamesMichael Haggerty, Sep 15, 2011
  11. 10/22 resolve_ref(): explicitly fail if a symlink is not readableMichael Haggerty, Sep 15, 2011
  12. 11/22 resolve_ref(): use prefixcmp()Michael Haggerty, Sep 15, 2011
  13. 12/22 resolve_ref(): only follow a symlink that contains a valid, normalized refnameMichael Haggerty, Sep 15, 2011
  14. 13/22 resolve_ref(): turn buffer into a proper string as soon as possibleMichael Haggerty, Sep 15, 2011
  15. Thomas RastSep 23, 2011
  16. Michael HaggertySep 23, 2011
  17. 1/1 get_sha1_hex(): do not read past a NUL characterMichael Haggerty, Sep 23, 2011
  18. Junio C HamanoSep 23, 2011
  19. Thomas RastOct 5, 2011
  20. Junio C HamanoOct 5, 2011
  21. 14/22 resolve_ref(): extract a function get_packed_ref()Michael Haggerty, Sep 15, 2011
  22. 15/22 resolve_ref(): do not follow incorrectly-formatted symbolic refsMichael Haggerty, Sep 15, 2011
  23. 16/22 remote: use xstrdup() instead of strdup()Michael Haggerty, Sep 15, 2011
  24. 17/22 remote: avoid passing NULL to read_ref()Michael Haggerty, Sep 15, 2011
  25. 18/22 resolve_ref(): verify that the input refname has the right formatMichael Haggerty, Sep 15, 2011
  26. 19/22 resolve_ref(): emit warnings for improperly-formatted referencesMichael Haggerty, Sep 15, 2011
  27. Jeff KingOct 11, 2011
  28. Junio C HamanoOct 11, 2011
  29. Junio C HamanoOct 11, 2011
  30. Re* [PATCH v3 19/22] resolve_ref(): emit warnings for improperly-formatted referencesJunio C Hamano, Oct 11, 2011
  31. Jeff KingOct 11, 2011
  32. Junio C HamanoOct 11, 2011
  33. Jeff KingOct 11, 2011
  34. Junio C HamanoOct 12, 2011
  35. Jeff KingOct 11, 2011
  36. Junio C HamanoOct 11, 2011
  37. Jeff KingOct 12, 2011
  38. Junio C HamanoOct 12, 2011
  39. Jeff KingOct 12, 2011
  40. 1/2 refs.c: move dwim_ref()/dwim_log() from sha1_name.cJunio C Hamano, Oct 12, 2011
  41. 2/2 Restrict ref-like names immediately below $GIT_DIRJunio C Hamano, Oct 12, 2011
  42. Michael HaggertyOct 12, 2011
  43. Junio C HamanoOct 12, 2011
  44. Michael HaggertyOct 12, 2011
  45. Junio C HamanoOct 12, 2011
  46. Junio C HamanoOct 19, 2011
  47. Junio C HamanoOct 19, 2011
  48. Michael HaggertyOct 19, 2011
  49. Junio C HamanoOct 19, 2011
  50. Junio C HamanoOct 19, 2011
  51. resolve_ref(): report breakage to the caller without warningJunio C Hamano, Oct 19, 2011
  52. Michael HaggertyOct 19, 2011
  53. Junio C HamanoOct 19, 2011
  54. Jeff KingOct 12, 2011
  55. Michael HaggertyOct 12, 2011
  56. Junio C HamanoOct 12, 2011
  57. Jeff KingOct 12, 2011
  58. 20/22 resolve_ref(): also treat a too-long SHA1 as invalidMichael Haggerty, Sep 15, 2011
  59. 21/22 resolve_ref(): expand documentationMichael Haggerty, Sep 15, 2011
  60. 22/22 add_ref(): verify that the refname is formatted correctlyMichael Haggerty, Sep 15, 2011

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.