In different contexts the question "Is it ok to delete a submodule?"
may be answered differently.
In 293ab15eea (submodule: teach rm to remove submodules unless they
contain a git directory, 2012-09-26) a case was made that we can safely
ignore ignored untracked files for removal as we explicitely ask for the
removal of the submodule.
In a later patch we want to remove submodules even when the user doesn't
explicitly ask for it (e.g. checking out a tree-ish in which the submodule
doesn't exist). In that case we want to be more careful when it comes
to deletion of untracked files. As of this patch it is unclear how this
will be implemented exactly, so we'll offer flags in which the caller
can specify how the different untracked files ought to be handled.
As the flags allow the function to not die on an error when spawning
a child process, we need to find an appropriate return code for the
case when the child process could not be started. As in that case we
cannot tell if the submodule is ok to remove, we'd want to return 'false'.
As only 0 is understood as false, rename the function to invert the
meaning, i.e. the return code of 0 signals the removal of the submodule
is fine, and other values can be used to return a more precise answer
what went wrong.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of constructing the NULL terminated array ourselves, we
should make use of the argv_array infrastructure.
While at it, adapt the error messages to reflect the actual invocation.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a submodule has its git dir inside the working dir, the submodule
support for checkout that we plan to add in a later patch will fail.
Add functionality to migrate the git directory to be absorbed
into the superprojects git directory.
The newly added code in this patch is structured such that other areas of
Git can also make use of it. The code in the submodule--helper is a mere
wrapper and option parser for the function
`absorb_git_dir_into_superproject`, that takes care of embedding the
submodules git directory into the superprojects git dir. That function
makes use of the more abstract function for this use case
`relocate_gitdir`, which can be used by e.g. the worktree code eventually
to move around a git directory.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
That function was primarily used by submodule code, but the function
itself is not inherently about submodules. In the next patch we'll
introduce relocate_git_dir, which can be used by worktrees as well,
so find a neutral middle ground in dir.h.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current caller of connect_work_tree_and_git_dir passes
an absolute path for the `git_dir` parameter. In the future patch
we will also pass in relative path for `git_dir`. Extend the functionality
of connect_work_tree_and_git_dir to take relative paths for parameters.
We could work around this in the future patch by computing the absolute
path for the git_dir in the calling site, however accepting relative
paths for either parameter makes the API for this function much harder
to misuse.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Codepaths involved in interacting alternate object store have
been cleaned up.
* jk/alt-odb-cleanup:
alternates: use fspathcmp to detect duplicates
sha1_file: always allow relative paths to alternates
count-objects: report alternates via verbose mode
fill_sha1_file: write into a strbuf
alternates: store scratch buffer as strbuf
fill_sha1_file: write "boring" characters
alternates: use a separate scratch space
alternates: encapsulate alt->base munging
alternates: provide helper for allocating alternate
alternates: provide helper for adding to alternates list
link_alt_odb_entry: refactor string handling
link_alt_odb_entry: handle normalize_path errors
t5613: clarify "too deep" recursion tests
t5613: do not chdir in main process
t5613: whitespace/style cleanups
t5613: use test_must_fail
t5613: drop test_valid_repo function
t5613: drop reachable_via function
The submodule code wants to temporarily add an alternate
object store to our in-memory alt_odb list, but does it
manually. Let's provide a helper so it can reuse the code in
link_alt_odb_entry().
While we're adding our new add_to_alternates_memory(), let's
document add_to_alternates_file(), as the two are related.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs
instead of taking detours through find_unique_abbrev() and its static
buffer. This is shorter in most cases and a bit more efficient.
The changes here are not easily handled by a semantic patch because
they involve removing temporary variables and deconstructing format
strings for strbuf_addf().
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code clean-up with help from coccinelle tool continues.
* rs/cocci:
coccicheck: make transformation for strbuf_addf(sb, "...") more precise
use strbuf_add_unique_abbrev() for adding short hashes, part 2
use strbuf_addstr() instead of strbuf_addf() with "%s", part 2
gitignore: ignore output files of coccicheck make target
Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs
instead of taking detours through find_unique_abbrev() and its static
buffer. This is shorter and a bit more efficient.
1eb47f167d already converted six cases,
this patch covers three more.
A semantic patch for Coccinelle is included for easier checking for
new cases that might be introduced in the future.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Replace uses of strbuf_addf() for adding strings with more lightweight
strbuf_addstr() calls. This is shorter and makes the intent clearer.
bc57b9c0cc already converted three cases,
this patch covers two more.
A semantic patch for Coccinelle is included for easier checking for
new cases that might be introduced in the future.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The callbacks for iterating a sha1_array must have a void
return. This is unlike our usual for_each semantics, where
a callback may interrupt iteration and have its value
propagated. Let's switch it to the usual form, which will
enable its use in more places (e.g., where we are replacing
an existing iteration with a different data structure).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Having a submodule whose ".git" repository is somehow corrupt
caused a few commands that recurse into submodules loop forever.
* jc/submodule-anchor-git-dir:
submodule: avoid auto-discovery in prepare_submodule_repo_env()
The function is used to set up the environment variable used in a
subprocess we spawn in a submodule directory. The callers set up a
child_process structure, find the working tree path of one submodule
and set .dir field to it, and then use start_command() API to spawn
the subprocess like "status", "fetch", etc.
When this happens, we expect that the ".git" (either a directory or
a gitfile that points at the real location) in the current working
directory of the subprocess MUST be the repository for the submodule.
If this ".git" thing is a corrupt repository, however, because
prepare_submodule_repo_env() unsets GIT_DIR and GIT_WORK_TREE, the
subprocess will see ".git", thinks it is not a repository, and
attempt to find one by going up, likely to end up in finding the
repository of the superproject. In some codepaths, this will cause
a command run with the "--recurse-submodules" option to recurse
forever.
By exporting GIT_DIR=.git, disable the auto-discovery logic in the
subprocess, which would instead stop it and report an error.
The test illustrates existing problems in a few callsites of this
function. Without this fix, "git fetch --recurse-submodules", "git
status" and "git diff" keep recursing forever.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Teach git-diff and friends a new format for displaying the difference
of a submodule. The new format is an inline diff of the contents of the
submodule between the commit range of the update. This allows the user
to see the actual code change caused by a submodule update.
Add tests for the new format and option.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
A future patch is going to add a new submodule diff format which
displays an inline diff of the submodule changes. To make this easier,
and to ensure that both submodule diff formats use the same initial
header, factor out show_submodule_header() function which will print the
current submodule header line, and then leave the show_submodule_summary
function to lookup and print the submodule log format.
This does create one format change in that "(revision walker failed)"
will now be displayed on its own line rather than as part of the message
because we no longer perform this step directly in the header display
flow. However, this is a rare case as most causes of the failure will be
due to a missing commit which we already check for and avoid previously.
flow. However, this is a rare case and shouldn't impact much.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since we're going to be changing this function in a future patch, lets
go ahead and convert this to use object_id now.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently, do_submodule_path will attempt locating the .git directory by
using read_gitfile on <path>/.git. If this fails it just assumes the
<path>/.git is actually a git directory.
This is good because it allows for handling submodules which were cloned
in a regular manner first before being added to the superproject.
Unfortunately this fails if the <path> is not actually checked out any
longer, such as by removing the directory.
Fix this by checking if the directory we found is actually a gitdir. In
the case it is not, attempt to lookup the submodule configuration and
find the name of where it is stored in the .git/modules/ directory of
the superproject.
If we can't locate the submodule configuration, this might occur because
for example a submodule gitlink was added but the corresponding
.gitmodules file was not properly updated. A die() here would not be
pleasant to the users of submodule diff formats, so instead, modify
do_submodule_path() to return an error code:
- git_pathdup_submodule() returns NULL when we fail to find a path.
- strbuf_git_path_submodule() propagates the error code to the caller.
Modify the callers of these functions to check the error code and fail
properly. This ensures we don't attempt to use a bad path that doesn't
match the corresponding submodule.
Because this change fixes add_submodule_odb() to work even if the
submodule is not checked out, update the wording of the submodule log
diff format to correctly display that the submodule is "not initialized"
instead of "not checked out"
Add tests to ensure this change works as expected.
Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert struct diff_filespec's sha1 member to use a struct object_id
called "oid" instead. The following Coccinelle semantic patch was used
to implement this, followed by the transformations in object_id.cocci:
@@
struct diff_filespec o;
@@
- o.sha1
+ o.oid.hash
@@
struct diff_filespec *p;
@@
- p->sha1
+ p->oid.hash
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
There are two types of string_lists: those that own the
string memory, and those that don't. You can tell the
difference by the strdup_strings flag, and one should use
either STRING_LIST_INIT_DUP, or STRING_LIST_INIT_NODUP as an
initializer.
Historically, the normal all-zeros initialization has
corresponded to the NODUP case. Many sites use no
initializer at all, and that works as a shorthand for that
case. But for a reader of the code, it can be hard to
remember which is which. Let's be more explicit and actually
have each site declare which type it means to use.
This is a fairly mechanical conversion; I assumed each site
was correct as-is, and just switched them all to NODUP.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An earlier addition of "sanitize_submodule_env" with 14111fc4 (git:
submodule honor -c credential.* from command line, 2016-02-29)
turned out to be a convoluted no-op; implement what it wanted to do
correctly, and stop filtering settings given via "git -c var=val".
* jk/submodule-c-credential:
submodule: stop sanitizing config options
submodule: use prepare_submodule_repo_env consistently
submodule--helper: move config-sanitizing to submodule.c
submodule: export sanitized GIT_CONFIG_PARAMETERS
t5550: break submodule config test into multiple sub-tests
t5550: fix typo in $HTTPD_URL
The point of having a whitelist of command-line config
options to pass to submodules was two-fold:
1. It prevented obvious nonsense like using core.worktree
for multiple repos.
2. It could prevent surprise when the user did not mean
for the options to leak to the submodules (e.g.,
http.sslverify=false).
For case 1, the answer is mostly "if it hurts, don't do
that". For case 2, we can note that any such example has a
matching inverted surprise (e.g., a user who meant
http.sslverify=true to apply everywhere, but it didn't).
So this whitelist is probably not giving us any benefit, and
is already creating a hassle as people propose things to put
on it. Let's just drop it entirely.
Note that we still need to keep a special code path for
"prepare the submodule environment", because we still have
to take care to pass through $GIT_CONFIG_PARAMETERS (and
block the rest of the repo-specific environment variables).
We can do this easily from within the submodule shell
script, which lets us drop the submodule--helper option
entirely (and it's OK to do so because as a "--" program, it
is entirely a private implementation detail).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Before 14111fc (git: submodule honor -c credential.* from
command line, 2016-02-29), it was sufficient for code which
spawned a process in a submodule to just set the child
process's "env" field to "local_repo_env" to clear the
environment of any repo-specific variables.
That commit introduced a more complicated procedure, in
which we clear most variables but allow through sanitized
config. For C code, we used that procedure only for cloning,
but not for any of the programs spawned by submodule.c. As a
result, things like "git fetch --recurse-submodules" behave
differently than "git clone --recursive"; the former will
not pass through the sanitized config.
We can fix this by using prepare_submodule_repo_env()
everywhere in submodule.c.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
These functions should be used by any code which spawns a
submodule process, which may happen in submodule.c (e.g.,
for spawning fetch). Let's move them there and make them
public so that submodule--helper can continue to use them.
Since they're now public, let's also provide a basic overview
of their intended use.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
By having the `submodule init` functionality in C, we can reference it
easier from other parts in the code in later patches. The code is split
up to have one function to initialize one submodule and a calling function
that takes care of the rest, such as argument handling and translating the
arguments to the paths of the submodules.
This is the first submodule subcommand that is fully converted to C
except for the usage string, so this is actually removing a call to
the `submodule--helper list` function, which is supposed to be used in
this transition. Instead we'll make a direct call to `module_list_compute`.
An explanation why we need to edit the prefixes in cmd_update in
git-submodule.sh in this patch:
By having no processing in the shell part, we need to convey the notion
of wt_prefix and prefix to the C parts, which former patches punted on
and did the processing of displaying path in the shell.
`wt_prefix` used to hold the path from the repository root to the current
directory, e.g. wt_prefix would be t/ if the user invoked the
`git submodule` command in ~/repo/t and ~repo is the GIT_DIR.
`prefix` used to hold the relative path from the repository root to the
operation, e.g. if you have recursive submodules, the shell script would
modify the `prefix` in each recursive step by adding the submodule path.
We will pass `wt_prefix` into the C helper via `git -C <dir>` as that
will setup git in the directory the user actually called git-submodule.sh
from. The `prefix` will be passed in via the `--prefix` option.
Having `prefix` and `wt_prefix` relative to the GIT_DIR of the
calling superproject is unfortunate with this patch as the C code doesn't
know about a possible recursion from a superproject via `submodule update
--init --recursive`.
To fix this, we change the meaning of `wt_prefix` to point to the current
project instead of the superproject and `prefix` to include any relative
paths issues in the superproject. That way `prefix` will become the leading
part for displaying paths and `wt_prefix` will be empty in recursive
calls for now.
The new notion of `wt_prefix` and `prefix` still allows us to reconstruct
the calling directory in the superproject by just traveling reverse of
`prefix`.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git submodule" reports the paths of submodules the command
recurses into, but this was incorrect when the command was not run
from the root level of the superproject.
Any further comments? Otherwise will merge to 'next'.
* sb/submodule-path-misc-bugs: (600 commits)
t7407: make expectation as clear as possible
submodule update: test recursive path reporting from subdirectory
submodule update: align reporting path for custom command execution
submodule status: correct path handling in recursive submodules
submodule update --init: correct path handling in recursive submodules
submodule foreach: correct path display in recursive submodules
Git 2.8
Documentation: fix git-p4 AsciiDoc formatting
mingw: skip some tests in t9115 due to file name issues
t1300: fix the new --show-origin tests on Windows
t1300-repo-config: make it resilient to being run via 'sh -x'
config --show-origin: report paths with forward slashes
submodule: fix regression for deinit without submodules
l10n: pt_PT: Update and add new translations
l10n: ca.po: update translation
Git 2.8-rc4
Documentation: fix broken linkgit to git-config
Documentation: use ASCII quotation marks in git-p4
Revert "config.mak.uname: use clang for Mac OS X 10.6"
git-compat-util: st_add4: work around gcc 4.2.x compiler crash
...
Simplify the two callback functions that are triggered when the
child process terminates to avoid misuse of the child-process
structure that has already been cleaned up.
* sb/submodule-parallel-fetch:
run-command: do not pass child process data into callbacks
This allows to configure fetching and updating in parallel
without having the command line option.
This moved the responsibility to determine how many parallel processes
to start from builtin/fetch to submodule.c as we need a way to communicate
"The user did not specify the number of parallel processes in the command
line options" in the builtin fetch. The submodule code takes care of
the precedence (CLI > config > default).
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Currently submodule.<name>.update is only handled by git-submodule.sh.
C code will start to need to make use of that value as more of the
functionality of git-submodule.sh moves into library code in C.
Add the update field to 'struct submodule' and populate it so it can
be read as sm->update or from sm->update_command.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The expected way to pass data into the callback is to pass them via
the customizable callback pointer. The error reporting in
default_{start_failure, task_finished} is not user friendly enough, that
we want to encourage using the child data for such purposes.
Furthermore the struct child data is cleaned by the run-command API,
before we access them in the callbacks, leading to use-after-free
situations.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many codepaths forget to check return value from git_config_set();
the function is made to die() to make sure we do not proceed when
setting a configuration variable failed.
* ps/config-error:
config: rename git_config_set_or_die to git_config_set
config: rename git_config_set to git_config_set_gently
compat: die when unable to set core.precomposeunicode
sequencer: die on config error when saving replay opts
init-db: die on config errors when initializing empty repo
clone: die on config error in cmd_clone
remote: die on config error when manipulating remotes
remote: die on config error when setting/adding branches
remote: die on config error when setting URL
submodule--helper: die on config error when cloning module
submodule: die on config error when linking modules
branch: die on config error when editing branch description
branch: die on config error when unsetting upstream
branch: report errors in tracking branch setup
config: introduce set_or_die wrappers
Update various codepaths to avoid manually-counted malloc().
* jk/tighten-alloc: (22 commits)
ewah: convert to REALLOC_ARRAY, etc
convert ewah/bitmap code to use xmalloc
diff_populate_gitlink: use a strbuf
transport_anonymize_url: use xstrfmt
git-compat-util: drop mempcpy compat code
sequencer: simplify memory allocation of get_message
test-path-utils: fix normalize_path_copy output buffer size
fetch-pack: simplify add_sought_entry
fast-import: simplify allocation in start_packfile
write_untracked_extension: use FLEX_ALLOC helper
prepare_{git,shell}_cmd: use argv_array
use st_add and st_mult for allocation size computation
convert trivial cases to FLEX_ARRAY macros
use xmallocz to avoid size arithmetic
convert trivial cases to ALLOC_ARRAY
convert manual allocations to argv_array
argv-array: add detach function
add helpers for allocating flex-array structs
harden REALLOC_ARRAY and xcalloc against size_t overflow
tree-diff: catch integer overflow in combine_diff_path allocation
...
If our size computation overflows size_t, we may allocate a
much smaller buffer than we expected and overflow it. It's
probably impossible to trigger an overflow in most of these
sites in practice, but it is easy enough convert their
additions and multiplications into overflow-checking
variants. This may be fixing real bugs, and it makes
auditing the code easier.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When trying to connect a submodule with its corresponding
repository in '.git/modules' we try to set the core.worktree
setting in the submodule, which may fail due to an error
encountered in `git_config_set_in_file`.
The function is used in the git-mv command when trying to move a
submodule to another location. We already die when renaming a
file fails but do not pay attention to the case where updating
the connection between submodule and its repository fails. As
this leaves the repository in an inconsistent state, as well,
abort the program by dying early and presenting the failure to
the user.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a later patch we enable parallel processing of submodules, this
only adds the possibility for it. So this change should not change
any user facing behavior.
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "Pushing submodule <foo>" progress output correctly goes to
stderr, but "Fetching submodule <foo>" is going to stdout by
mistake. Fix it to write to stderr.
Noticed while trying to implement a parallel submodule fetch. When
this particular output line went to a different file descriptor, it
was buffered separately, resulting in wrongly interleaved output if
we copied it to the terminal naively.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Convert all instances of get_object_hash to use an appropriate reference
to the hash member of the oid member of struct object. This provides no
functional change, as it is essentially a macro substitution.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Jeff King <peff@peff.net>
struct object is one of the major data structures dealing with object
IDs. Convert it to use struct object_id instead of an unsigned char
array. Convert get_object_hash to refer to the new member as well.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Jeff King <peff@peff.net>
Convert most instances where the sha1 member of struct object is
dereferenced to use get_object_hash. Most instances that are passed to
functions that have versions taking struct object_id, such as
get_sha1_hex/get_oid_hex, or instances that can be trivially converted
to use struct object_id instead, are not converted.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Jeff King <peff@peff.net>
We peek objects from submodule's object store by linking it to the
list of alternate object databases, but the code to do so forgot to
correctly initialize the list.
* jk/initialization-fix-to-add-submodule-odb:
add_submodule_odb: initialize alt_odb list earlier
The add_submodule_odb function tries to add a submodule's
object store as an "alternate". It needs the existing list
to be initialized (from the objects/info/alternates file)
for two reasons:
1. We look for duplicates with the existing alternate
stores, but obviously this doesn't work if we haven't
loaded any yet.
2. We link our new entry into the list by prepending it to
alt_odb_list. But we do _not_ modify alt_odb_tail.
This variable starts as NULL, and is a signal to the
alt_odb code that the list has not yet been
initialized.
We then call read_info_alternates on the submodule (to
recursively load its alternates), which will try to
append to that tail, assuming it has been initialized.
This causes us to segfault if it is NULL.
This rarely comes up in practice, because we will have
initialized the alt_odb any time we do an object lookup. So
you can trigger this only when:
- you try to access a submodule (e.g., a diff with
diff.submodule=log)
- the access happens before any other object has been
accessed (e.g., because the diff is between the working
tree and the index)
- the submodule contains an alternates file (so we try to
add an entry to the NULL alt_odb_tail)
To fix this, we just need to call prepare_alt_odb at the
start of the function (and if we have already initialized,
it is a noop).
Note that we can remove the prepare_alt_odb call from the
end. It is guaranteed to be a noop, since we will have
called it earlier.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Many allocations that is manually counted (correctly) that are
followed by strcpy/sprintf have been replaced with a less error
prone constructs such as xstrfmt.
Macintosh-specific breakage was noticed and corrected in this
reroll.
* jk/war-on-sprintf: (70 commits)
name-rev: use strip_suffix to avoid magic numbers
use strbuf_complete to conditionally append slash
fsck: use for_each_loose_file_in_objdir
Makefile: drop D_INO_IN_DIRENT build knob
fsck: drop inode-sorting code
convert strncpy to memcpy
notes: document length of fanout path with a constant
color: add color_set helper for copying raw colors
prefer memcpy to strcpy
help: clean up kfmclient munging
receive-pack: simplify keep_arg computation
avoid sprintf and strcpy with flex arrays
use alloc_ref rather than hand-allocating "struct ref"
color: add overflow checks for parsing colors
drop strcpy in favor of raw sha1_to_hex
use sha1_to_hex_r() instead of strcpy
daemon: use cld->env_array when re-spawning
stat_tracking_info: convert to argv_array
http-push: use an argv_array for setup_revisions
fetch-pack: use argv_array for index-pack / unpack-objects
...
When we are allocating a struct with a FLEX_ARRAY member, we
generally compute the size of the array and then sprintf or
strcpy into it. Normally we could improve a dynamic allocation
like this by using xstrfmt, but it doesn't work here; we
have to account for the size of the rest of the struct.
But we can improve things a bit by storing the length that
we use for the allocation, and then feeding it to xsnprintf
or memcpy, which makes it more obvious that we are not
writing more than the allocated number of bytes.
It would be nice if we had some kind of helper for
allocating generic flex arrays, but it doesn't work that
well:
- the call signature is a little bit unwieldy:
d = flex_struct(sizeof(*d), offsetof(d, path), fmt, ...);
You need offsetof here instead of just writing to the
end of the base size, because we don't know how the
struct is packed (partially this is because FLEX_ARRAY
might not be zero, though we can account for that; but
the size of the struct may actually be rounded up for
alignment, and we can't know that).
- some sites do clever things, like over-allocating because
they know they will write larger things into the buffer
later (e.g., struct packed_git here).
So we're better off to just write out each allocation (or
add type-specific helpers, though many of these are one-off
allocations anyway).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Functions which directly operate submodule's object database do not
handle the case when the submodule is linked worktree (which are
introduced in c7b3a3d2fe). Instead of fixing the path calculation use
already existing strbuf_git_path_submodule() function without changing
overall behaviour. Then it will be possible to modify only that function
whenever we need to change real location of submodule's repository
content.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Max Kirillov <max@max630.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
