summaryrefslogtreecommitdiff
path: root/pack-write.c
Commit message (Collapse)AuthorAgeFilesLines
* object-store-ll.h: split this header out of object-store.hElijah Newren2023-05-161-0/+1
| | | | | | | | | | | | | | | | | The vast majority of files including object-store.h did not need dir.h nor khash.h. Split the header into two files, and let most just depend upon object-store-ll.h, while letting the two callers that need it depend on the full object-store.h. After this patch: $ git grep -h include..object-store | sort | uniq -c 2 #include "object-store.h" 129 #include "object-store-ll.h" Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* repository: remove unnecessary include of path.hElijah Newren2023-05-161-0/+1
| | | | | | | | This also made it clear that several .c files that depended upon path.h were missing a #include for it; add the missing includes while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'tb/enable-cruft-packs-by-default'Junio C Hamano2023-04-281-6/+7
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When "gc" needs to retain unreachable objects, packing them into cruft packs (instead of exploding them into loose object files) has been offered as a more efficient option for some time. Now the use of cruft packs has been made the default and no longer considered an experimental feature. * tb/enable-cruft-packs-by-default: repository.h: drop unused `gc_cruft_packs` builtin/gc.c: make `gc.cruftPacks` enabled by default t/t9300-fast-import.sh: prepare for `gc --cruft` by default t/t6500-gc.sh: add additional test cases t/t6500-gc.sh: refactor cruft pack tests t/t6501-freshen-objects.sh: prepare for `gc --cruft` by default t/t5304-prune.sh: prepare for `gc --cruft` by default builtin/gc.c: ignore cruft packs with `--keep-largest-pack` builtin/repack.c: fix incorrect reference to '-C' pack-write.c: plug a leak in stage_tmp_packfiles()
| * pack-write.c: plug a leak in stage_tmp_packfiles()Taylor Blau2023-04-181-6/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function `stage_tmp_packfiles()` generates a filename to use for staging the contents of what will become the pack's ".mtimes" file. The name is generated in `write_mtimes_file()` and the result is returned back to `stage_tmp_packfiles()` which uses it to rename the temporary file into place via `rename_tmp_packfiles()`. `write_mtimes_file()` returns a `const char *`, indicating that callers are not expected to free its result (similar to, e.g., `oid_to_hex()`). But callers are expected to free its result, so this return type is incorrect. Change the function's signature to return a non-const `char *`, and free it at the end of `stage_tmp_packfiles()`. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'tb/pack-revindex-on-disk'Junio C Hamano2023-04-271-0/+2
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The on-disk reverse index that allows mapping from the pack offset to the object name for the object stored at the offset has been enabled by default. * tb/pack-revindex-on-disk: t: invert `GIT_TEST_WRITE_REV_INDEX` config: enable `pack.writeReverseIndex` by default pack-revindex: introduce `pack.readReverseIndex` pack-revindex: introduce GIT_TEST_REV_INDEX_DIE_ON_DISK pack-revindex: make `load_pack_revindex` take a repository t5325: mark as leak-free pack-write.c: plug a leak in stage_tmp_packfiles()
| * | pack-write.c: plug a leak in stage_tmp_packfiles()Taylor Blau2023-04-131-0/+2
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function `stage_tmp_packfiles()` generates a filename to use for staging the contents of what will become the pack's ".rev" file. The name is generated in `write_rev_file_order()` (via its caller `write_rev_file()`) in a string buffer, and the result is returned back to `stage_tmp_packfiles()` which uses it to rename the temporary file into place via `rename_tmp_packfiles()`. That name is not visible outside of `stage_tmp_packfiles()`, so it can (and should) be `free()`'d at the end of that function. We can't free it in `rename_tmp_packfile()` since not all of its `source` arguments are unreachable after calling it. Instead, simply free() `rev_tmp_name` at the end of `stage_tmp_packfiles()`. (Note that the same leak exists for `mtimes_tmp_name`, but we do not address it in this commit). Signed-off-by: Taylor Blau <me@ttaylorr.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | treewide: remove double forward declaration of read_in_fullElijah Newren2023-04-111-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | cache.h's nature of a dumping ground of includes prevented it from being included in some compat/ files, forcing us into a workaround of having a double forward declaration of the read_in_full() function (see commit 14086b0a13 ("compat/pread.c: Add a forward declaration to fix a warning", 2007-11-17)). Now that we have moved functions like read_in_full() from cache.h to wrapper.h, and wrapper.h isn't littered with unrelated and scary #defines, get rid of the extra forward declaration and just have compat/pread.c include wrapper.h. Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | treewide: remove unnecessary cache.h inclusionElijah Newren2023-04-111-1/+1
| | | | | | | | | | | | | | | | | | | | | | Several files were including cache.h solely to get other headers, such as trace.h and trace2.h. Since the last few commits have modified files to make these dependencies more explicit, the inclusion of cache.h is no longer needed in several cases. Remove it. Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | treewide: be explicit about dependence on pack-revindex.hElijah Newren2023-04-111-0/+1
|/ | | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Acked-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* environment.h: move declarations for environment.c functions from cache.hElijah Newren2023-03-211-0/+1
| | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* treewide: be explicit about dependence on gettext.hElijah Newren2023-03-211-0/+1
| | | | | | | | | | | | | | Dozens of files made use of gettext functions, without explicitly including gettext.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include gettext.h if they are using it. However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an include of gettext.h, it was left out to avoid conflicting with an in-flight topic. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* cache.h: remove dependence on hex.h; make other files include it explicitlyElijah Newren2023-02-231-0/+1
| | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* git: remove duplicate includesSeija Kijin2022-12-151-1/+0
| | | | | | | | These files are already included; we do not need to include them again Signed-off-by: Seija Kijin <doremylover123@gmail.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pack-write: drop always-NULL parameterDerrick Stolee2022-06-161-11/+6
| | | | | | | | | write_mtimes_file() takes an mtimes parameter as its first option, but the only caller passes a NULL constant. Drop this parameter to simplify logic. This can be reverted if that parameter is needed in the future. Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pack-mtimes: support writing pack .mtimes filesTaylor Blau2022-05-261-0/+77
| | | | | | | | | | Now that the `.mtimes` format is defined, supplement the pack-write API to be able to conditionally write an `.mtimes` file along with a pack by setting an additional flag and passing an oidmap that contains the timestamps corresponding to each object in the pack. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* chunk-format.h: extract oid_version()Taylor Blau2022-05-261-13/+2
| | | | | | | | | | | | | | | | | | | | | There are three definitions of an identical function which converts `the_hash_algo` into either 1 (for SHA-1) or 2 (for SHA-256). There is a copy of this function for writing both the commit-graph and multi-pack-index file, and another inline definition used to write the .rev header. Consolidate these into a single definition in chunk-format.h. It's not clear that this is the best header to define this function in, but it should do for now. (Worth noting, the .rev caller expects a 4-byte unsigned, but the other two callers work with a single unsigned byte. The consolidated version uses the latter type, and lets the compiler widen it when required). Another caller will be added in a subsequent patch. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pack-write: pass 'struct packing_data' to 'stage_tmp_packfiles'Taylor Blau2022-05-261-0/+1
| | | | | | | | | | | | | | | | This structure will be used to communicate the per-object mtimes when writing a cruft pack. Here, we need the full packing_data structure because the mtime information is stored in an array there, not on the individual object_entry's themselves (to avoid paying the overhead in structure width for operations which do not generate a cruft pack). We haven't passed this information down before because one of the two callers (in bulk-checkin.c) does not have a packing_data structure at all. In that case (where no cruft pack will be generated), NULL is passed instead. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* core.fsync: introduce granular fsync control infrastructureNeeraj Singh2022-03-101-6/+7
| | | | | | | | | | | | | | | | | | | | | | | | This commit introduces the infrastructure for the core.fsync configuration knob. The repository components we want to sync are identified by flags so that we can turn on or off syncing for specific components. If core.fsyncObjectFiles is set and the core.fsync configuration also includes FSYNC_COMPONENT_LOOSE_OBJECT, we will fsync any loose objects. This picks the strictest data integrity behavior if core.fsync and core.fsyncObjectFiles are set to conflicting values. This change introduces the currently unused fsync_component helper, which will be used by a later patch that adds fsyncing to the refs backend. Actual configuration and documentation of the fsync components list are in other patches in the series to separate review of the underlying mechanism from the policy of how it's configured. Helped-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'tb/pack-finalize-ordering'Junio C Hamano2021-09-201-29/+28
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The order in which various files that make up a single (conceptual) packfile has been reevaluated and straightened up. This matters in correctness, as an incomplete set of files must not be shown to a running Git. * tb/pack-finalize-ordering: pack-objects: rename .idx files into place after .bitmap files pack-write: split up finish_tmp_packfile() function builtin/index-pack.c: move `.idx` files into place last index-pack: refactor renaming in final() builtin/repack.c: move `.idx` files into place last pack-write.c: rename `.idx` files after `*.rev` pack-write: refactor renaming in finish_tmp_packfile() bulk-checkin.c: store checksum directly pack.h: line-wrap the definition of finish_tmp_packfile()
| * pack-write: split up finish_tmp_packfile() functionÆvar Arnfjörð Bjarmason2021-09-091-9/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Split up the finish_tmp_packfile() function and use the split-up version in pack-objects.c in preparation for moving the step of renaming the *.idx file later as part of a function change. Since the only other caller of finish_tmp_packfile() was in bulk-checkin.c, and it won't be needing a change to its *.idx renaming, provide a thin wrapper for the old function as a static function in that file. If other callers end up needing the simpler version it could be moved back to "pack-write.c" and "pack.h". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * pack-write.c: rename `.idx` files after `*.rev`Taylor Blau2021-09-091-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We treat the presence of an `.idx` file as the indicator of whether or not it's safe to use a packfile. But `finish_tmp_packfile()` (which is responsible for writing the pack and moving the temporary versions of all of its auxiliary files into place) is inconsistent about the write order. Specifically, it moves the `.rev` file into place after the `.idx`, leaving open the possibility to open a pack which looks "ready" (because the `.idx` file exists and is readable) but appears momentarily to not have a `.rev` file. This causes Git to fall back to generating the pack's reverse index in memory. Though racy, this amounts to an unnecessary slow-down at worst, and doesn't affect the correctness of the resulting reverse index. Close this race by moving the .rev file into place before moving the .idx file into place. This still leaves the issue of `.idx` files being renamed into place before the auxiliary `.bitmap` file is renamed when in pack-object.c's write_pack_file() "write_bitmap_index" is true. That race will be addressed in subsequent commits. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * pack-write: refactor renaming in finish_tmp_packfile()Ævar Arnfjörð Bjarmason2021-09-091-21/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor the renaming in finish_tmp_packfile() into a helper function. The callers are now expected to pass a "name_buffer" ending in "pack-OID." instead of the previous "pack-", we then append "pack", "idx" or "rev" to it. By doing the strbuf_setlen() in rename_tmp_packfile() we reuse the buffer and avoid the repeated allocations we'd get if that function had its own temporary "struct strbuf". This approach of reusing the buffer does make the last user in pack-object.c's write_pack_file() slightly awkward, since we needlessly do a strbuf_setlen() before calling strbuf_release() for consistency. In subsequent changes we'll move that bitmap writing code around, so let's not skip the strbuf_setlen() now. The previous strbuf_reset() idiom originated with 5889271114a (finish_tmp_packfile():use strbuf for pathname construction, 2014-03-03), which in turn was a minimal adjustment of pre-strbuf code added in 0e990530ae (finish_tmp_packfile(): a helper function, 2011-10-28). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ab/reverse-midx-optim'Junio C Hamano2021-09-151-0/+3
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | The code that optionally creates the *.rev reverse index file has been optimized to avoid needless computation when it is not writing the file out. * ab/reverse-midx-optim: pack-write: skip *.rev work when not writing *.rev
| * | pack-write: skip *.rev work when not writing *.revÆvar Arnfjörð Bjarmason2021-09-071-0/+3
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a performance regression introduced in a587b5a786 (pack-write.c: extract 'write_rev_file_order', 2021-03-30) and stop needlessly allocating the "pack_order" array and sorting it with "pack_order_cmp()", only to throw that work away when we discover that we're not writing *.rev files after all. This redundant work was not present in the original version of this code added in 8ef50d9958 (pack-write.c: prepare to write 'pack-*.rev' files, 2021-01-25). There we'd call write_rev_file() from e.g. finish_tmp_packfile(), but we'd "return NULL" early in write_rev_file() if not doing a "WRITE_REV" or "WRITE_REV_VERIFY". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | use xopen() to handle fatal open(2) failuresRené Scharfe2021-08-251-6/+2
|/ | | | | | | | | Add and apply a semantic patch for using xopen() instead of calling open(2) and die() or die_errno() explicitly. This makes the error messages more consistent and shortens the code. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'tb/reverse-midx'Junio C Hamano2021-04-081-11/+25
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | An on-disk reverse-index to map the in-pack location of an object back to its object name across multiple packfiles is introduced. * tb/reverse-midx: midx.c: improve cache locality in midx_pack_order_cmp() pack-revindex: write multi-pack reverse indexes pack-write.c: extract 'write_rev_file_order' pack-revindex: read multi-pack reverse indexes Documentation/technical: describe multi-pack reverse indexes midx: make some functions non-static midx: keep track of the checksum midx: don't free midx_name early midx: allow marking a pack as preferred t/helper/test-read-midx.c: add '--show-objects' builtin/multi-pack-index.c: display usage on unrecognized command builtin/multi-pack-index.c: don't enter bogus cmd_mode builtin/multi-pack-index.c: split sub-commands builtin/multi-pack-index.c: define common usage with a macro builtin/multi-pack-index.c: don't handle 'progress' separately builtin/multi-pack-index.c: inline 'flags' with options
| * pack-write.c: extract 'write_rev_file_order'Taylor Blau2021-04-011-11/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Existing callers provide the reverse index code with an array of 'struct pack_idx_entry *'s, which is then sorted by pack order (comparing the offsets of each object within the pack). Prepare for the multi-pack index to write a .rev file by providing a way to write the reverse index without an array of pack_idx_entry (which the MIDX code does not have). Instead, callers can invoke 'write_rev_index_positions()', which takes an array of uint32_t's. The ith entry in this array specifies the ith object's (in index order) position within the pack (in pack order). Expose this new function for use in a later patch, and rewrite the existing write_rev_file() in terms of this new function. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jt/transfer-fsck-across-packs'Junio C Hamano2021-03-011-1/+7
|\ \ | |/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The approach to "fsck" the incoming objects in "index-pack" is attractive for performance reasons (we have them already in core, inflated and ready to be inspected), but fundamentally cannot be applied fully when we receive more than one pack stream, as a tree object in one pack may refer to a blob object in another pack as ".gitmodules", when we want to inspect blobs that are used as ".gitmodules" file, for example. Teach "index-pack" to emit objects that must be inspected later and check them in the calling "fetch-pack" process. * jt/transfer-fsck-across-packs: fetch-pack: print and use dangling .gitmodules fetch-pack: with packfile URIs, use index-pack arg http-fetch: allow custom index-pack args http: allow custom index-pack args
| * fetch-pack: print and use dangling .gitmodulesJonathan Tan2021-02-221-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Teach index-pack to print dangling .gitmodules links after its "keep" or "pack" line instead of declaring an error, and teach fetch-pack to check such lines printed. This allows the tree side of the .gitmodules link to be in one packfile and the blob side to be in another without failing the fsck check, because it is now fetch-pack which checks such objects after all packfiles have been downloaded and indexed (and not index-pack on an individual packfile, as it is before this commit). Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pack-write.c: prepare to write 'pack-*.rev' filesTaylor Blau2021-01-251-1/+119
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch prepares for callers to be able to write reverse index files to disk. It adds the necessary machinery to write a format-compliant .rev file from within 'write_rev_file()', which is called from 'finish_tmp_packfile()'. Similar to the process by which the reverse index is computed in memory, these new paths also have to sort a list of objects by their offsets within a packfile. These new paths use a qsort() (as opposed to a radix sort), since our specialized radix sort requires a full revindex_entry struct per object, which is more memory than we need to allocate. The qsort is obviously slower, but the theoretical slowdown would require a repository with a large amount of objects, likely implying that the time spent in, say, pack-objects during a repack would dominate the overall runtime. Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pack-write: die on error in write_promisor_file()Christian Couder2021-01-141-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | write_promisor_file() already uses xfopen(), so it would die if the file cannot be opened for writing. To be consistent with this behavior and not overlook issues, let's also die if there are errors when we are actually writing to the file. Suggested-by: Jeff King <peff@peff.net> Suggested-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | fetch-pack: refactor writing promisor fileChristian Couder2021-01-121-0/+12
|/ | | | | | | | | | | | | | | | Let's replace the 2 different pieces of code that write a promisor file in 'builtin/repack.c' and 'fetch-pack.c' with a new function called 'write_promisor_file()' in 'pack-write.c' and 'pack.h'. This might also help us in the future, if we want to put back the ref names and associated hashes that were in the promisor files we are repacking in 'builtin/repack.c' as suggested by a NEEDSWORK comment just above the code we are refactoring. Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'rs/hashwrite-be64'Junio C Hamano2020-11-251-4/+1
|\ | | | | | | | | | | | | | | | | Code simplification. * rs/hashwrite-be64: pack-write: use hashwrite_be64() midx: use hashwrite_be64() csum-file: add hashwrite_be64()
| * pack-write: use hashwrite_be64()René Scharfe2020-11-121-4/+1
| | | | | | | | | | | | | | | | | | Call hashwrite_be64() to write a 64-bit value instead of open-coding it using htonl() and hashwrite(). This shortens the code, gets rid of a buffer and several magic numbers, and makes the intent clearer. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pack-write: use hashwrite_be32() instead of double-buffering arrayRené Scharfe2020-11-011-3/+1
|/ | | | | | | | | | hashwrite() already buffers writes, so pass the fanout table entries individually via hashwrite_be32(), which also does the endianess conversion for us. This avoids a memory copy, shortens the code and reduces the number of magic numbers. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* pack-write: use hashwrite_be32() in write_idx_file()René Scharfe2020-09-191-8/+4
| | | | | | | | Call hashwrite_be32() instead of open-coding it. This shortens the code a bit and makes it easier to read. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'jb/doc-packfile-name' into masterJunio C Hamano2020-07-301-3/+2
|\ | | | | | | | | | | | | Doc update. * jb/doc-packfile-name: pack-write/docs: update regarding pack naming
| * pack-write/docs: update regarding pack namingJohannes Berg2020-07-221-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The index-pack documentation explicitly states that the pack name is derived from the sorted list of object names, but since commit 1190a1acf800 ("pack-objects: name pack files after trailer hash") that isn't true anymore. Be less explicit in the docs as to what the exact output is, and just say that it's whatever goes into the pack name. Also update a comment on write_idx_file() since it no longer modifies the sha1 variable (it's const now anyway), as noted by Junio. Fixes: 1190a1acf800 ("pack-objects: name pack files after trailer hash") Signed-off-by: Johannes Berg <johannes@sipsolutions.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pack-write: use hash_to_hex when writing checksumsbrian m. carlson2019-08-191-4/+4
| | | | | | | | | | | | | | | | Pack checksums always use the current hash algorithm in use, so switch from sha1_to_hex to hash_to_hex. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | convert "hashcmp() != 0" to "!hasheq()"Jeff King2018-08-291-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | This rounds out the previous three patches, covering the inequality logic for the "hash" variant of the functions. As with the previous three, the accompanying code changes are the mechanical result of applying the coccinelle patch; see those patches for more discussion. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | convert "oidcmp() == 0" to oideq()Jeff King2018-08-291-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | csum-file: refactor finalize_hashfile() methodDerrick Stolee2018-04-021-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | If we want to use a hashfile on the temporary file for a lockfile, then we need finalize_hashfile() to fully write the trailing hash but also keep the file descriptor open. Do this by adding a new CSUM_HASH_IN_STREAM flag along with a functional change that checks this flag before writing the checksum to the stream. This differs from previous behavior since it would be written if either CSUM_CLOSE or CSUM_FSYNC is provided. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | csum-file: rename hashclose() to finalize_hashfile()Derrick Stolee2018-04-021-2/+2
| | | | | | | | | | | | | | | | | | | | | | The hashclose() method behaves very differently depending on the flags parameter. In particular, the file descriptor is not always closed. Perform a simple rename of "hashclose()" to "finalize_hashfile()" in preparation for functional changes. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | csum-file: rename sha1file to hashfilebrian m. carlson2018-02-021-16/+16
| | | | | | | | | | | | | | | | | | Rename struct sha1file to struct hashfile, along with all of its related functions. The transformation in this commit was made by global search-and-replace. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pack-write: switch various SHA-1 values to abstract formsbrian m. carlson2018-02-021-24/+25
| | | | | | | | | | | | | | | | | | Convert various uses of hardcoded 20- and 40-based numbers to use the_hash_algo, along with direct calls to SHA-1. Adjust the names of variables to refer to "hash" instead of "sha1". Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | avoid looking at errno for short read_in_full() returnsJeff King2017-09-271-1/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When a caller tries to read a particular set of bytes via read_in_full(), there are three possible outcomes: 1. An error, in which case -1 is returned and errno is set. 2. A short read, in which fewer bytes are returned and errno is unspecified (we never saw a read error, so we may have some random value from whatever syscall failed last). 3. The full read completed successfully. Many callers handle cases 1 and 2 together by just checking the result against the requested size. If their combined error path looks at errno (e.g., by calling die_errno), they may report a nonsense value. Let's fix these sites by having them distinguish between the two error cases. That avoids the random errno confusion, and lets us give more detailed error messages. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | pack: convert struct pack_idx_entry to struct object_idbrian m. carlson2017-05-081-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Convert struct pack_idx_entry to use struct object_id by changing the definition and applying the following semantic patch, plus the standard object_id transforms: @@ struct pack_idx_entry E1; @@ - E1.sha1 + E1.oid.hash @@ struct pack_idx_entry *E1; @@ - E1->sha1 + E1->oid.hash Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | odb_mkstemp: write filename into strbufJeff King2017-03-281-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The odb_mkstemp() function expects the caller to provide a fixed buffer to write the resulting tempfile name into. But it creates the template using snprintf without checking the return value. This means we could silently truncate the filename. In practice, it's unlikely that the truncation would end in the template-pattern that mkstemp needs to open the file. So we'd probably end up failing either way, unless the path was specially crafted. The simplest fix would be to notice the truncation and die. However, we can observe that most callers immediately xstrdup() the result anyway. So instead, let's switch to using a strbuf, which is easier for them (and isn't a big deal for the other 2 callers, who can just strbuf_release when they're done with it). Note that many of the callers used static buffers, but this was purely to avoid putting a large buffer on the stack. We never passed the static buffers out of the function, so there's no complicated memory handling we need to change. Signed-off-by: Jeff King <peff@peff.net>
* | do not check odb_mkstemp return value for errorsJeff King2017-03-281-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The odb_mkstemp function does not return an error; it dies on failure instead. But many of its callers compare the resulting descriptor against -1 and die themselves. Mostly this is just pointless, but it does raise a question when looking at the callers: if they show the results of the "template" buffer after a failure, what's in it? The answer is: it doesn't matter, because it cannot happen. So let's make that clear by removing the bogus error checks. In bitmap_writer_finish(), we can drop the error-handling code entirely. In the other two cases, it's shared with the open() in another code path; we can just move the error-check next to that open() call. And while we're at it, let's flesh out the function's docstring a bit to make the error behavior clear. Signed-off-by: Jeff King <peff@peff.net>
* | encode_in_pack_object_header: respect output buffer lengthJeff King2017-03-241-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The encode_in_pack_object_header() writes a variable-length header to an output buffer, but it doesn't actually know long the buffer is. At first glance, this looks like it might be possible to overflow. In practice, this is probably impossible. The smallest buffer we use is 10 bytes, which would hold the header for an object up to 2^67 bytes. Obviously we're not likely to see such an object, but we might worry that an object could lie about its size (causing us to overflow before we realize it does not actually have that many bytes). But the argument is passed as a uintmax_t. Even on systems that have __int128 available, uintmax_t is typically restricted to 64-bit by the ABI. So it's unlikely that a system exists where this could be exploited. Still, it's easy enough to use a normal out/len pair and make sure we don't write too far. That protects the hypothetical 128-bit system, makes it harder for callers to accidentally specify a too-small buffer, and makes the resulting code easier to audit. Note that the one caller in fast-import tried to catch such a case, but did so _after_ the call (at which point we'd have already overflowed!). This check can now go away. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
View Lorry Controller Status