summaryrefslogtreecommitdiff
path: root/notes.c
AgeCommit message (Collapse)Author
2021-04-27hash: provide per-algorithm null OIDsbrian m. carlson
Up until recently, object IDs did not have an algorithm member, only a hash. Consequently, it was possible to share one null (all-zeros) object ID among all hash algorithms. Now that we're going to be handling objects from multiple hash algorithms, it's important to make sure that all object IDs have a correct algorithm field. Introduce a per-algorithm null OID, and add it to struct hash_algo. Introduce a wrapper function as well, and use it everywhere we used to use the null_oid constant. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-27hash: set, copy, and use algo field in struct object_idbrian m. carlson
Now that struct object_id has an algorithm field, we should populate it. This will allow us to handle object IDs in any supported algorithm and distinguish between them. Ensure that the field is written whenever we write an object ID by storing it explicitly every time we write an object. Set values for the empty blob and tree values as well. In addition, use the algorithm field to compare object IDs. Note that because we zero-initialize struct object_id in many places throughout the codebase, we default to the default algorithm in cases where the algorithm field is zero rather than explicitly initialize all of those locations. This leads to a branch on every comparison, but the alternative is to compare the entire buffer each time and padding the buffer for SHA-1. That alternative ranges up to 3.9% worse than this approach on the perf t0001, t1450, and t1451. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-27Always use oidread to read into struct object_idbrian m. carlson
In the future, we'll want oidread to automatically set the hash algorithm member for an object ID we read into it, so ensure we use oidread instead of hashcpy everywhere we're copying a hash value into a struct object_id. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-14use CALLOC_ARRAYRené Scharfe
Add and apply a semantic patch for converting code that open-codes CALLOC_ARRAY to use it instead. It shortens the code and infers the element size automatically. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-30Merge branch 'na/notes-displayref-is-not-boolean'Junio C Hamano
Config parser fix for "git notes". * na/notes-displayref-is-not-boolean: t3301: test proper exit response to no-value notes.displayRef. notes.c: fix a segfault in notes_display_config()
2020-11-23notes.c: fix a segfault in notes_display_config()Nate Avers
If notes.displayRef is configured with no value[1], control should be returned to the caller when notes.c:notes_display_config() checks if 'v' is NULL. Otherwise, both git log --notes and git diff-tree --notes will subsequently segfault when refs.h:has_glob_specials() calls strpbrk() with a NULL first argument. [1] Examples: .git/config: [notes] displayRef $ git -c notes.displayRef [...] Signed-off-by: Nate Avers <nate@roosteregg.cc> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-17Merge branch 'rs/strbuf-insertstr'Junio C Hamano
Code clean-up. * rs/strbuf-insertstr: mailinfo: don't insert header prefix for handle_content_type() strbuf: add and use strbuf_insertstr()
2020-02-14Merge branch 'jh/notes-fanout-fix'Junio C Hamano
The code to automatically shrink the fan-out in the notes tree had an off-by-one bug, which has been killed. * jh/notes-fanout-fix: notes.c: fix off-by-one error when decreasing notes fanout t3305: check notes fanout more carefully and robustly
2020-02-10strbuf: add and use strbuf_insertstr()René Scharfe
Add a function for inserting a C string into a strbuf. Use it throughout the source to get rid of magic string length constants and explicit strlen() calls. Like strbuf_addstr(), implement it as an inline function to avoid the implicit strlen() calls to cause runtime overhead. Helped-by: Taylor Blau <me@ttaylorr.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-04notes.c: fix off-by-one error when decreasing notes fanoutJohan Herland
As noted in the previous commit, the nature of the fanout heuristic in the notes code causes the exact point at which we increase or decrease the notes fanout to vary with the objects being annotated. Since the object ids generated by the test environment are deterministic (by design), the notes generated and tested by t3305 are always the same, and we therefore happen to see the same fanout behavior from one run to the next. Coincidentally, if we were to change the test environment slightly (say by making a test commit on an unrelated branch before we start the t3305 test proper), we not only see the fanout switch happen at different points, we also manage to trigger a _bug_ in the notes code where the fanout 1 -> 0 switch is not applied uniformly across the notes tree, but instead yields a notes tree like this: ... bdeafb301e44b0e4db0f738a2d2a7beefdb70b70 bff2d39b4f7122bd4c5caee3de353a774d1e632a d3/8ec8f851adf470131178085bfbaab4b12ad2a7 e0b173960431a3e692ae929736df3c9b73a11d5b eb3c3aede523d729990ac25c62a93eb47c21e2e3 ... The bug occurs when we are writing out a notes tree with a newly decreased fanout, and the notes tree contains unexpanded subtrees that should be consolidated into the parent tree as a consequence of the decreased fanout): Subtrees that happen to sit at an _even_ level in the internal notes 16-tree structure (in other words: subtrees whose path - "d3" in the example above - is unique in the first nibble - i.e. there are no other note paths that start with "d") are _not_ unpacked as part of the tree writeout. This error will repeat itself in subsequent note trees until the subtree is forced to be unpacked. In t3305 this only happens when the d38ec8f8 note is itself removed from the tree. The error is not severe (no information is lost, and the notes code is able to read/decode this tree and manipulate it correctly), but this is nonetheless a bug in the current implementation that should be fixed. That said, fixing the off-by-one error is not without complications: We must take into account that the load_subtree() call from for_each_note_helper() (that is now done to correctly unpack the subtree while we're writing out the notes tree) may end up inserting unpacked non-notes into the linked list of non_note entries held by the struct notes_tree. Since we are in the process of writing out the notes tree, this linked list is currently in the process of being traversed by write_each_non_note_until(). The unpacked non-notes are necessarily inserted between the last non-note we wrote out, and the next non-note to be written. Hence, we cannot simply hold the next_non_note to write in struct write_each_note_data (as we would then silently skip these newly inserted notes), but must instead always follow the ->next pointer from the last non-note we wrote. (This part was caught by an existing test in t3304.) Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de> Cc: Brian M. Carlson <sandals@crustytoothpaste.net> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-31C: use skip_prefix() to avoid hardcoded string lengthJunio C Hamano
We often skip an optional prefix in a string with a hardcoded constant, e.g. if (starts_with(string, "prefix")) string += 6; which is less error prone when written skip_prefix(string, "prefix", &string); Note that this changes a few error messages from "git reflog expire --expire=nonsense.timestamp", which used to complain by saying '--expire=nonsense.timestamp' is not a valid timestamp but with this change, we say 'nonsense.timestamp' is not a valid timestamp which is more technically correct (the string with --expire= as a prefix obviously cannot be a valid timestamp, but the error is about the part of the input without that prefix). Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-25Merge branch 'dl/format-patch-notes-config-fixup'Junio C Hamano
"git format-patch" can take a set of configured format.notes values to specify which notes refs to use in the log message part of the output. The behaviour of this was not consistent with multiple --notes command line options, which has been corrected. * dl/format-patch-notes-config-fixup: notes.h: fix typos in comment notes: break set_display_notes() into smaller functions config/format.txt: clarify behavior of multiple format.notes format-patch: move git_config() before repo_init_revisions() format-patch: use --notes behavior for format.notes notes: extract logic into set_display_notes() notes: create init_display_notes() helper notes: rename to load_display_notes()
2019-12-13notes: break set_display_notes() into smaller functionsDenton Liu
In 8164c961e1 (format-patch: use --notes behavior for format.notes, 2019-12-09), we introduced set_display_notes() which was a monolithic function with three mutually exclusive branches. Break the function up into three small and simple functions that each are only responsible for one task. This family of functions accepts an `int *show_notes` instead of returning a value suitable for assignment to `show_notes`. This is for two reasons. First of all, this guarantees that the external `show_notes` variable changes in lockstep with the `struct display_notes_opt`. Second, this prompts future developers to be careful about doing something meaningful with this value. In fact, a NULL check is intentionally omitted because causing a segfault here would tell the future developer that they are meant to use the value for something meaningful. One alternative was making the family of functions accept a `struct rev_info *` instead of the `struct display_notes_opt *`, since the former contains the `show_notes` field as well. This does not work because we have to call git_config() before repo_init_revisions(). However, if we had a `struct rev_info`, we'd need to initialize it before it gets assigned values from git_config(). As a result, we break the circular dependency by having standalone `int show_notes` and `struct display_notes_opt notes_opt` variables which temporarily hold values from git_config() until the values are copied over to `rev`. To implement this change, we need to get a pointer to `rev_info::show_notes`. Unfortunately, this is not possible with bitfields and only direct-assignment is possible. Change `rev_info::show_notes` to a non-bitfield int so that we can get its address. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09notes: extract logic into set_display_notes()Denton Liu
Instead of open coding the logic that tweaks the variables in `struct display_notes_opt` within handle_revision_opt(), abstract away the logic into set_display_notes() so that it can be reused. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09notes: create init_display_notes() helperDenton Liu
We currently open code the initialization for revs->notes_opt. Abstract this away into a helper function so that the logic can be reused in a future commit. This is slightly wasteful as we memset the struct twice but this is only run once so it shouldn't have any major effect. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09notes: rename to load_display_notes()Denton Liu
According to the function comment, init_display_notes() was supposed to "Load the notes machinery for displaying several notes trees." Rename this function to load_display_notes() so that its use is more accurately represented. This is done because, in a future commit, we will reuse the name init_display_notes(). Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-30Merge branch 'mh/notes-duplicate-entries'Junio C Hamano
A few implementation fixes in the notes API. * mh/notes-duplicate-entries: notes: avoid potential use-after-free during insertion notes: avoid leaking duplicate entries
2019-08-26notes: avoid potential use-after-free during insertionJeff King
The note_tree_insert() function may free the leaf_node struct we pass in (e.g., if it's a duplicate, or if it needs to be combined with an existing note). Most callers are happy with this, as they assume that ownership of the struct is handed off. But in load_subtree(), if we see an error we'll use the handed-off struct's key_oid to generate the die() message, potentially accessing freed memory. We can easily fix this by instead using the original oid that we copied into the leaf_node struct. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-26notes: avoid leaking duplicate entriesMike Hommey
When add_note is called multiple times with the same key/value pair, the leaf_node it creates is leaked by notes_tree_insert. Signed-off-by: Mike Hommey <mh@glandium.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-27tree-walk.c: remove the_repo from get_tree_entry()Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-27tree-walk.c: remove the_repo from fill_tree_descriptor()Nguyễn Thái Ngọc Duy
While at there, clean up the_repo usage in builtin/merge-tree.c a tiny bit. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-08Merge branch 'en/merge-directory-renames'Junio C Hamano
"git merge-recursive" backend recently learned a new heuristics to infer file movement based on how other files in the same directory moved. As this is inherently less robust heuristics than the one based on the content similarity of the file itself (rather than based on what its neighbours are doing), it sometimes gives an outcome unexpected by the end users. This has been toned down to leave the renamed paths in higher/conflicted stages in the index so that the user can examine and confirm the result. * en/merge-directory-renames: merge-recursive: switch directory rename detection default merge-recursive: give callers of handle_content_merge() access to contents merge-recursive: track information associated with directory renames t6043: fix copied test description to match its purpose merge-recursive: switch from (oid,mode) pairs to a diff_filespec merge-recursive: cleanup handle_rename_* function signatures merge-recursive: track branch where rename occurred in rename struct merge-recursive: remove ren[12]_other fields from rename_conflict_info merge-recursive: shrink rename_conflict_info merge-recursive: move some struct declarations together merge-recursive: use 'ci' for rename_conflict_info variable name merge-recursive: rename locals 'o' and 'a' to 'obuf' and 'abuf' merge-recursive: rename diff_filespec 'one' to 'o' merge-recursive: rename merge_options argument from 'o' to 'opt' Use 'unsigned short' for mode, like diff_filespec does
2019-04-08Use 'unsigned short' for mode, like diff_filespec doesElijah Newren
struct diff_filespec defines mode to be an 'unsigned short'. Several other places in the API which we'd like to interact with using a diff_filespec used a plain unsigned (or unsigned int). This caused problems when taking addresses, so switch to unsigned short. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-01notes: replace sha1_to_hexbrian m. carlson
Replace the uses of sha1_to_hex in this function with hash_to_hex to allow the use of SHA-256 as well. Rename some variables since this code is no longer limited to SHA-1. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-01notes: make hash size independentbrian m. carlson
Switch out various uses of the GIT_SHA1_* constants with GIT_MAX_* constants for allocations and the_hash_algo for general parsing. Update a comment to no longer be SHA-1 specific. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-15tree-walk: store object_id in a separate memberbrian m. carlson
When parsing a tree, we read the object ID directly out of the tree buffer. This is normally fine, but such an object ID cannot be used with oidcpy, which copies GIT_MAX_RAWSZ bytes, because if we are using SHA-1, there may not be that many bytes to copy. Instead, store the object ID in a separate struct member. Since we can no longer efficiently compute the path length, store that information as well in struct name_entry. Ensure we only copy the object ID into the new buffer if the path length is nonzero, as some callers will pass us an empty path with no object ID following it, and we will not want to read past the end of the buffer. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29convert "oidcmp() != 0" to "!oideq()"Jeff King
This is the flip side of the previous two patches: checking for a non-zero oidcmp() can be more strictly expressed as inequality. Like those patches, we write "!= 0" in the coccinelle transformation, which covers by isomorphism the more common: if (oidcmp(E1, E2)) As with the previous two patches, this patch can be achieved almost entirely by running "make coccicheck"; the only differences are manual line-wrap fixes to match the original code. There is one thing to note for anybody replicating this, though: coccinelle 1.0.4 seems to miss the case in builtin/tag.c, even though it's basically the same as all the others. Running with 1.0.7 does catch this, so presumably it's just a coccinelle bug that was fixed in the interim. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29convert "hashcmp() == 0" to hasheq()Jeff King
This is the partner patch to the previous one, but covering the "hash" variants instead of "oid". Note that our coccinelle rule is slightly more complex to avoid triggering the call in hasheq(). I didn't bother to add a new rule to convert: - hasheq(E1->hash, E2->hash) + oideq(E1, E2) Since these are new functions, there won't be any such existing callers. And since most of the code is already using oideq, we're not likely to introduce new ones. We might still see "!hashcmp(E1->hash, E2->hash)" from topics in flight. But because our new rule comes after the existing ones, that should first get converted to "!oidcmp(E1, E2)" and then to "oideq(E1, E2)". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-29convert "oidcmp() == 0" to oideq()Jeff King
Using the more restrictive oideq() should, in the long run, give the compiler more opportunities to optimize these callsites. For now, this conversion should be a complete noop with respect to the generated code. The result is also perhaps a little more readable, as it avoids the "zero is equal" idiom. Since it's so prevalent in C, I think seasoned programmers tend not to even notice it anymore, but it can sometimes make for awkward double negations (e.g., we can drop a few !!oidcmp() instances here). This patch was generated almost entirely by the included coccinelle patch. This mechanical conversion should be completely safe, because we check explicitly for cases where oidcmp() is compared to 0, which is what oideq() is doing under the hood. Note that we don't have to catch "!oidcmp()" separately; coccinelle's standard isomorphisms make sure the two are treated equivalently. I say "almost" because I did hand-edit the coccinelle output to fix up a few style violations (it mostly keeps the original formatting, but sometimes unwraps long lines). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-16object-store: move object access functions to object-store.hStefan Beller
This should make these functions easier to find and cache.h less overwhelming to read. In particular, this moves: - read_object_file - oid_object_info - write_object_file As a result, most of the codebase needs to #include object-store.h. In this patch the #include is only added to files that would fail to compile otherwise. It would be better to #include wherever identifiers from the header are used. That can happen later when we have better tooling for it. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-14sha1_file: convert read_sha1_file to struct object_idbrian m. carlson
Convert read_sha1_file to take a pointer to struct object_id and rename it read_object_file. Do the same for read_sha1_file_extended. Convert one use in grep.c to use the new function without any other code change, since the pointer being passed is a void pointer that is already initialized with a pointer to struct object_id. Update the declaration and definitions of the modified functions, and apply the following semantic patch to convert the remaining callers: @@ expression E1, E2, E3; @@ - read_sha1_file(E1.hash, E2, E3) + read_object_file(&E1, E2, E3) @@ expression E1, E2, E3; @@ - read_sha1_file(E1->hash, E2, E3) + read_object_file(E1, E2, E3) @@ expression E1, E2, E3, E4; @@ - read_sha1_file_extended(E1.hash, E2, E3, E4) + read_object_file_extended(&E1, E2, E3, E4) @@ expression E1, E2, E3, E4; @@ - read_sha1_file_extended(E1->hash, E2, E3, E4) + read_object_file_extended(E1, E2, E3, E4) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-14tree-walk: convert tree entry functions to object_idbrian m. carlson
Convert get_tree_entry and find_tree_entry to take pointers to struct object_id. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-30sha1_file: convert write_sha1_file to object_idPatryk Obara
Convert the definition and declaration of write_sha1_file to struct object_id and adjust usage of this function. This commit also converts static function write_sha1_file_prepare, as it is closely related. Rename these functions to write_object_file and write_object_file_prepare respectively. Replace sha1_to_hex, hashcpy and hashclr with their oid equivalents wherever possible. Signed-off-by: Patryk Obara <patryk.obara@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-30notes: convert write_notes_tree to object_idPatryk Obara
Convert the definition and declaration of write_notes_tree to struct object_id and adjust usage of this function. Additionally, improve style of small part of this function, as old formatting made it hard to understand at glance what this part of code is doing. Signed-off-by: Patryk Obara <patryk.obara@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-30notes: convert combine_notes_* to object_idPatryk Obara
Convert the definition and declarations of combine_notes_* functions to struct object_id and adjust usage of these functions. Signed-off-by: Patryk Obara <patryk.obara@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-09Merge branch 'rs/hex-to-bytes-cleanup'Junio C Hamano
Code cleanup. * rs/hex-to-bytes-cleanup: sha1_file: use hex_to_bytes() http-push: use hex_to_bytes() notes: move hex_to_bytes() to hex.c and export it
2017-11-01notes: move hex_to_bytes() to hex.c and export itRené Scharfe
Make the function for converting pairs of hexadecimal digits to binary available to other call sites. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-16refs: convert read_ref and read_ref_full to object_idbrian m. carlson
All but two of the call sites already have parameters using the hash parameter of struct object_id, so convert them to take a pointer to the struct directly. Also convert refs_read_refs_full, the underlying implementation. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-19Merge branch 'mh/notes-cleanup'Junio C Hamano
Code clean-up. * mh/notes-cleanup: load_subtree(): check that `prefix_len` is in the expected range load_subtree(): declare some variables to be `size_t` hex_to_bytes(): simpler replacement for `get_oid_hex_segment()` get_oid_hex_segment(): don't pad the rest of `oid` load_subtree(): combine some common code get_oid_hex_segment(): return 0 on success load_subtree(): only consider blobs to be potential notes load_subtree(): check earlier whether an internal node is a tree entry load_subtree(): separate logic for internal vs. terminal entries load_subtree(): fix incorrect comment load_subtree(): reduce the scope of some local variables load_subtree(): remove unnecessary conditional notes: make GET_NIBBLE macro more robust
2017-09-08load_subtree(): check that `prefix_len` is in the expected rangeMichael Haggerty
This value, which is stashed in the last byte of an object_id hash, gets handed around a lot. So add a sanity check before using it in `load_subtree()`. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26load_subtree(): declare some variables to be `size_t`Michael Haggerty
* `prefix_len` * `path_len` * `i` It's good hygiene. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26hex_to_bytes(): simpler replacement for `get_oid_hex_segment()`Michael Haggerty
Now that `get_oid_hex_segment()` does less, it makes sense to rename it and simplify its semantics: * Instead of a `hex_len` parameter, which was the number of hex characters (and had to be even), use a `len` parameter, which is the number of resulting bytes. This removes then need for the check that `hex_len` is even and to divide it by two to determine the number of bytes. For good hygiene, declare the `len` parameter to be `size_t` instead of `unsigned int`. * Change the order of the arguments to the more traditional (dst, src, len). * Rename the function to `hex_to_bytes()`. * Remove a loop variable: just count `len` down instead. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26get_oid_hex_segment(): don't pad the rest of `oid`Michael Haggerty
Remove the feature of `get_oid_hex_segment()` that it pads the rest of the `oid` argument with zeros. Instead, do this at the caller who needs it. This makes the functionality of this function more coherent and removes the need for its `oid_len` argument. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26load_subtree(): combine some common codeMichael Haggerty
Write the length into `object_oid` (before copying) rather than `l->key_oid` (after copying). Then combine some code from the two `if` blocks. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26get_oid_hex_segment(): return 0 on successMichael Haggerty
Nobody cares about the return value of get_oid_hex_segment() except to check whether it failed. So just return 0 on success. And while we're updating its docstring, update it for some argument renaming that happened a while ago. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26load_subtree(): only consider blobs to be potential notesMichael Haggerty
The old code converted any entry whose path constituted a full SHA-1 as a leaf node, without regard for the type of the entry. But only blobs can be notes. So treat entries whose paths *look like* notes paths but that are not blobs as non-notes. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26load_subtree(): check earlier whether an internal node is a tree entryMichael Haggerty
If an entry is not a tree entry, then it cannot possibly be an internal node. But the old code checked this condition only after allocating a leaf_node object and therefore leaked that memory. Instead, check before even entering this branch of the code. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26load_subtree(): separate logic for internal vs. terminal entriesMichael Haggerty
There are only two legitimate notes path components: * A hexadecimal string that fills the rest of the SHA-1 * A two-digit hexadecimal string that constitutes another internal node. So handle those two cases at the top level, and reject others as non-notes without trying to parse them. The logic separation also simplifies upcoming changes. This prevents us from leaking memory for a leaf_node in the case of wrong-sized paths. There are still memory leaks in this code; they will be fixed in upcoming commits. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26load_subtree(): fix incorrect commentMichael Haggerty
This comment was added in 851c2b3791 (Teach notes code to properly preserve non-notes in the notes tree, 2010-02-13) when the corresponding code was added. But I believe it was incorrect even then. The condition `path_len != 2` a dozen lines up prevents a path like "dead/beef" from being converted to "de/ad/beef", and indeed the test added in commit 851c2b3 verifies that this case works correctly. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-26load_subtree(): reduce the scope of some local variablesMichael Haggerty
Declare the variables inside the loop, to make it more obvious that their values are not carried across loop iterations. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>