Commit Graph

273 Commits

Author SHA1 Message Date
Johannes Schindelin
2d4dcf210e setup_discovered_git_dir(): plug memory leak
The setup_explicit_git_dir() function does not take custody of the string
passed as first parameter; we have to release it if we turned the value of
git_dir into an absolute path.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08 12:18:19 +09:00
Johannes Schindelin
da6f847559 setup_bare_git_dir(): help static analysis
Coverity reported a memory leak in this function. However, it can only
be called once, as setup_git_directory() changes global state and hence
is not reentrant.

Mark the variable as static to indicate that this is a singleton.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08 12:18:19 +09:00
Junio C Hamano
3736c92558 Merge branch 'bw/recurse-submodules-relative-fix'
A few commands that recently learned the "--recurse-submodule"
option misbehaved when started from a subdirectory of the
superproject.

* bw/recurse-submodules-relative-fix:
  ls-files: fix bug when recursing with relative pathspec
  ls-files: fix typo in variable name
  grep: fix bug when recursing with relative pathspec
  setup: allow for prefix to be passed to git commands
  grep: fix help text typo
2017-03-30 14:07:15 -07:00
Jeff King
e4da43b1f0 prefix_filename: return newly allocated string
The prefix_filename() function returns a pointer to static
storage, which makes it easy to use dangerously. We already
fixed one buggy caller in hash-object recently, and the
calls in apply.c are suspicious (I didn't dig in enough to
confirm that there is a bug, but we call the function once
in apply_all_patches() and then again indirectly from
parse_chunk()).

Let's make it harder to get wrong by allocating the return
value. For simplicity, we'll do this even when the prefix is
empty (and we could just return the original file pointer).
That will cause us to allocate sometimes when we wouldn't
otherwise need to, but this function isn't called in
performance critical code-paths (and it already _might_
allocate on any given call, so a caller that cares about
performance is questionable anyway).

The downside is that the callers need to remember to free()
the result to avoid leaking. Most of them already used
xstrdup() on the result, so we know they are OK. The
remainder have been converted to use free() as appropriate.

I considered retaining a prefix_filename_unsafe() for cases
where we know the static lifetime is OK (and handling the
cleanup is awkward). This is only a handful of cases,
though, and it's not worth the mental energy in worrying
about whether the "unsafe" variant is OK to use in any
situation.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 11:18:41 -07:00
Jeff King
116fb64e43 prefix_filename: drop length parameter
This function takes the prefix as a ptr/len pair, but in
every caller the length is exactly strlen(ptr). Let's
simplify the interface and just take the string. This saves
callers specifying it (and in some cases handling a NULL
prefix).

In a handful of cases we had the length already without
calling strlen, so this is technically slower. But it's not
likely to matter (after all, if the prefix is non-empty
we'll allocate and copy it into a buffer anyway).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 11:12:53 -07:00
Junio C Hamano
a0393a298f Merge branch 'js/early-config'
The start-up sequence of "git" needs to figure out some configured
settings before it finds and set itself up in the location of the
repository and was quite messy due to its "chicken-and-egg" nature.
The code has been restructured.

* js/early-config:
  setup.c: mention unresolved problems
  t1309: document cases where we would want early config not to die()
  setup_git_directory_gently_1(): avoid die()ing
  t1309: test read_early_config()
  read_early_config(): really discover .git/
  read_early_config(): avoid .git/config hack when unneeded
  setup: make read_early_config() reusable
  setup: introduce the discover_git_directory() function
  setup_git_directory_1(): avoid changing global state
  setup: prepare setup_discovered_git_dir() for the root directory
  setup_git_directory(): use is_dir_sep() helper
  t7006: replace dubious test
2017-03-17 13:50:28 -07:00
Brandon Williams
b58a68c1c1 setup: allow for prefix to be passed to git commands
In a future patch child processes which act on submodules need a little
more context about the original command that was invoked.  This patch
teaches git to use the prefix stored in `GIT_INTERNAL_TOPLEVEL_PREFIX`
instead of the prefix that was potentally found during the git directory
setup process.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-17 11:54:50 -07:00
Johannes Schindelin
5c4003ca3f setup.c: mention unresolved problems
During the review of the `early-config` patch series, two issues
have been identified that have been with us forever.  Mark the
identified problems for later so that we do not forget them.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-14 14:24:16 -07:00
Johannes Schindelin
01017dce54 setup_git_directory_gently_1(): avoid die()ing
This function now has a new caller in addition to setup_git_directory():
the newly introduced discover_git_directory(). That function wants to
discover the current .git/ directory, and in case of a corrupted one
simply pretend that there is none to be found.

Example: if a stale .git file exists in the parent directory, and the
user calls `git -p init`, we want Git to simply *not* read any
repository config for the pager (instead of aborting with a message that
the .git file is corrupt).

Let's actually pretend that there was no GIT_DIR to be found in that case
when being called from discover_git_directory(), but keep the previous
behavior (i.e. to die()) for the setup_git_directory() case.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-14 14:24:16 -07:00
Johannes Schindelin
16ac8b8db6 setup: introduce the discover_git_directory() function
We modified the setup_git_directory_gently_1() function earlier to make
it possible to discover the GIT_DIR without changing global state.

However, it is still a bit cumbersome to use if you only need to figure
out the (possibly absolute) path of the .git/ directory. Let's just
provide a convenient wrapper function with an easier signature that
*just* discovers the .git/ directory.

We will use it in a subsequent patch to fix the early config.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-14 14:24:16 -07:00
Johannes Schindelin
ce9b8aab5d setup_git_directory_1(): avoid changing global state
For historical reasons, Git searches for the .git/ directory (or the
.git file) by changing the working directory successively to the parent
directory of the current directory, until either anything was found or
until a ceiling or a mount point is hit.

Further global state may be changed in case a .git/ directory was found.

We do have a use case, though, where we would like to find the .git/
directory without having any global state touched, though: when we read
the early config e.g. for the pager or for alias expansion.

Let's just move all of code that changes any global state out of the
function `setup_git_directory_gently_1()` into
`setup_git_directory_gently()`.

In subsequent patches, we will use the _1() function in a new
`discover_git_directory()` function that we will then use for the early
config code.

Note: the new loop is a *little* tricky, as we have to handle the root
directory specially: we cannot simply strip away the last component
including the slash, as the root directory only has that slash. To remedy
that, we introduce the `min_offset` variable that holds the minimal length
of an absolute path, and using that to special-case the root directory,
including an early exit before trying to find the parent of the root
directory.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-14 14:24:16 -07:00
Johannes Schindelin
df380d58ec setup: prepare setup_discovered_git_dir() for the root directory
Currently, the offset parameter (indicating what part of the cwd
parameter corresponds to the current directory after discovering the
.git/ directory) is set to 0 when we are running in the root directory.

However, in the next patches we will avoid changing the current working
directory while searching for the .git/ directory, meaning that the
offset corresponding to the root directory will have to be 1 to reflect
that this directory is characterized by the path "/" (and not "").

So let's make sure that setup_discovered_git_directory() only tries to
append the trailing slash to non-root directories.

Note: the setup_bare_git_directory() does not need a corresponding
change, as it does not want to return a prefix.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-14 14:24:04 -07:00
Junio C Hamano
ba37c92df9 Merge branch 'js/realpath-pathdup-fix'
Git v2.12 was shipped with an embarrassing breakage where various
operations that verify paths given from the user stopped dying when
seeing an issue, and instead later triggering segfault.

* js/realpath-pathdup-fix:
  real_pathdup(): fix callsites that wanted it to die on error
  t1501: demonstrate NULL pointer access with invalid GIT_WORK_TREE
2017-03-12 23:21:33 -07:00
Junio C Hamano
fc32293502 Merge branch 'rs/strbuf-add-real-path'
An helper function to make it easier to append the result from
real_path() to a strbuf has been added.

* rs/strbuf-add-real-path:
  strbuf: add strbuf_add_real_path()
  cocci: use ALLOC_ARRAY
2017-03-10 13:24:23 -08:00
Johannes Schindelin
ce83eadd9a real_pathdup(): fix callsites that wanted it to die on error
In 4ac9006f83 (real_path: have callers use real_pathdup and
strbuf_realpath, 2016-12-12), we changed the xstrdup(real_path())
pattern to use real_pathdup() directly.

The problem with this change is that real_path() calls
strbuf_realpath() with die_on_error = 1 while real_pathdup() calls
it with die_on_error = 0. Meaning that in cases where real_path()
causes Git to die() with an error message, real_pathdup() is silent
and returns NULL instead.

The callers, however, are ill-prepared for that change, as they expect
the return value to be non-NULL (and otherwise the function died
with an appropriate error message).

Fix this by extending real_pathdup()'s signature to accept the
die_on_error flag and simply pass it through to strbuf_realpath(),
and then adjust all callers after a careful audit whether they would
handle NULLs well.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-08 14:38:41 -08:00
Johannes Schindelin
6c1e654437 setup_git_directory(): use is_dir_sep() helper
It is okay in practice to test for forward slashes in the output of
getcwd(), because we go out of our way to convert backslashes to forward
slashes in getcwd()'s output on Windows.

Still, the correct way to test for a dir separator is by using the
helper function we introduced for that very purpose. It also serves as a
good documentation what the code tries to do (not "how").

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-07 15:18:55 -08:00
René Scharfe
33ad9ddd0b strbuf: add strbuf_add_real_path()
Add a function for appending the canonized absolute pathname of a given
path to a strbuf.  It keeps the existing contents intact, as expected of
a function of the strbuf_add() family, while avoiding copying the result
if the given strbuf is empty.  It's more consistent with the rest of the
strbuf API than strbuf_realpath(), which it's wrapping.

Also add a semantic patch demonstrating its intended usage and apply it
to the current tree.  Using strbuf_add_real_path() instead of calling
strbuf_addstr() and real_path() avoids an extra copy to a static buffer.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-27 11:02:06 -08:00
Stefan Beller
5f29433f1c cache.h: expose the dying procedure for reading gitlinks
In a later patch we want to react to only a subset of errors, defaulting
the rest to die as usual. Separate the block that takes care of dying
into its own function so we have easy access to it.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-26 11:00:58 -08:00
Stefan Beller
40d9632514 setup: add gentle version of resolve_git_dir
This follows a93bedada (setup: add gentle version of read_gitfile,
2015-06-09), and assumes the same reasoning. resolve_git_dir is unsuited
for speculative calls, so we want to use the gentle version to find out
about potential errors.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-26 11:00:24 -08:00
Brandon Williams
4ac9006f83 real_path: have callers use real_pathdup and strbuf_realpath
Migrate callers of real_path() who duplicate the retern value to use
real_pathdup or strbuf_realpath.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-12 15:22:32 -08:00
Vasco Almeida
2ff30e67d9 i18n: setup: mark error messages for translation
Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-09 12:44:59 -07:00
Vasco Almeida
ab33a76ec5 i18n: setup: mark strings for translation
Update tests that compare the strings newly marked for translation to
succeed when running under GETTEXT_POISON.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 15:45:48 -07:00
Junio C Hamano
3f80d16c1c Merge branch 'jc/xstrfmt-null-with-prec-0'
* jc/xstrfmt-null-with-prec-0:
  setup.c: do not feed NULL to "%.*s" even with precision 0
2016-04-22 15:45:08 -07:00
Junio C Hamano
907c416534 Merge branch 'jk/check-repository-format'
The repository set-up sequence has been streamlined (the biggest
change is that there is no longer git_config_early()), so that we
do not attempt to look into refs/* when we know we do not have a
Git repository.

* jk/check-repository-format:
  verify_repository_format: mark messages for translation
  setup: drop repository_format_version global
  setup: unify repository version callbacks
  init: use setup.c's repo version verification
  setup: refactor repo format reading and verification
  config: drop git_config_early
  check_repository_format_gently: stop using git_config_early
  lazily load core.sharedrepository
  wrap shared_repository global in get/set accessors
  setup: document check_repository_format()
2016-04-13 14:12:28 -07:00
Junio C Hamano
24041d6be5 setup.c: do not feed NULL to "%.*s" even with precision 0
A recent update 75faa45a (replace trivial malloc + sprintf / strcpy
calls with xstrfmt, 2015-09-24) rewrote

	prepare an empty buffer
	if (len)
        	append the first len bytes of "prefix" to the buffer
	append "path" to the buffer

that computed "path", optionally prefixed by "prefix", into

	xstrfmt("%.*s%s", len, prefix, path);

However, passing a NULL pointer to the printf(3) family of functions
to format it with %s conversion, even with the precision set to 0,
i.e.

	xstrfmt("%.*s", 0, NULL)

yields undefined results, at least on some platforms.

Avoid this problem by substituting prefix with "" when len==0, as
prefix can legally be NULL in that case.  This would mimick the
intent of the original code better.

Reported-by: Tom G. Christensen <tgc@jupiterrise.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-07 12:40:15 -07:00
Jeff King
274db840b4 verify_repository_format: mark messages for translation
These messages are human-readable and should be translated.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-11 15:02:24 -08:00
Jeff King
c90e5293d1 setup: drop repository_format_version global
Nobody reads this anymore, and they're not likely to; the
interesting thing is whether or not we passed
check_repository_format(), and possibly the individual
"extension" variables.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-11 15:02:24 -08:00
Jeff King
652f18ee87 setup: unify repository version callbacks
Once upon a time, check_repository_format_gently would parse
the config with a single callback, and that callback would
set up a bunch of global variables. But now that we have
separate workdirs, we have to be more careful. Commit
31e26eb (setup.c: support multi-checkout repo setup,
2014-11-30) introduced a reduced callback which omits some
values like core.worktree. In the "main" callback we call
the reduced one, and then add back in the missing variables.

Now that we have split the config-parsing from the munging
of the global variables, we can do it all with a single
callback, and keep all of the "are we in a separate workdir"
logic together.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-11 15:02:23 -08:00
Jeff King
2cc7c2c737 setup: refactor repo format reading and verification
When we want to know if we're in a git repository of
reasonable vintage, we can call check_repository_format_gently(),
which does three things:

  1. Reads the config from the .git/config file.

  2. Verifies that the version info we read is sane.

  3. Writes some global variables based on this.

There are a few things we could improve here.

One is that steps 1 and 3 happen together. So if the
verification in step 2 fails, we still clobber the global
variables. This is especially bad if we go on to try another
repository directory; we may end up with a state of mixed
config variables.

The second is there's no way to ask about the repository
version for anything besides the main repository we're in.
git-init wants to do this, and it's possible that we would
want to start doing so for submodules (e.g., to find out
which ref backend they're using).

We can improve both by splitting the first two steps into
separate functions. Now check_repository_format_gently()
calls out to steps 1 and 2, and does 3 only if step 2
succeeds.

Note that the public interface for read_repository_format()
and what check_repository_format_gently() needs from it are
not quite the same, leading us to have an extra
read_repository_format_1() helper. The extra needs from
check_repository_format_gently() will go away in a future
patch, and we can simplify this then to just the public
interface.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-11 15:02:23 -08:00
Jeff King
21627f9b6d check_repository_format_gently: stop using git_config_early
There's a chicken-and-egg problem with using the regular
git_config during the repository setup process. We get
around it here by using a special interface that lets us
specify the per-repo config, and avoid calling
git_pathdup().

But this interface doesn't actually make sense. It will look
in the system and per-user config, too; we definitely would
not want to accept a core.repositoryformatversion from
there.

The git_config_from_file interface is a better match, as it
lets us look at a single file.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-11 15:02:22 -08:00
Jeff King
ae5f67763b lazily load core.sharedrepository
The "shared_repository" config is loaded as part of
check_repository_format_version, but it's not quite like the
other values we check there. Something like
core.repositoryformatversion only makes sense in per-repo
config, but core.sharedrepository can be set in a per-user
config (e.g., to make all "git init" invocations shared by
default).

So it would make more sense as part of git_default_config.
Commit 457f06d (Introduce core.sharedrepository, 2005-12-22)
says:

  [...]the config variable is set in the function which
  checks the repository format. If this were done in
  git_default_config instead, a lot of programs would need
  to be modified to call git_config(git_default_config)
  first.

This is still the case today, but we have one extra trick up
our sleeve. Now that we have the git_configset
infrastructure, it's not so expensive for us to ask for a
single value. So we can simply lazy-load it on demand.

This should be OK to do in general. There are some problems
with loading config before setup_git_directory() is called,
but we shouldn't be accessing the value before then (if we
were, then it would already be broken, as the variable would
not have been set by check_repository_format_version!). The
trickiest caller is git-init, but it handles the values
manually itself.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-11 15:02:19 -08:00
Jeff King
7875acb6ec wrap shared_repository global in get/set accessors
It would be useful to control access to the global
shared_repository, so that we can lazily load its config.
The first step to doing so is to make sure all access
goes through a set of functions.

This step is purely mechanical, and should result in no
change of behavior.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-11 15:02:17 -08:00
Jeff King
4b0d1eebe9 setup: document check_repository_format()
This function's interface is rather enigmatic, so let's
document it further.

While we're here, let's also drop the return value. It will
always either be "0" or the function will die (consequently,
neither of its two callers bothered to check the return).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-11 15:02:13 -08:00
Jeff King
f1c126bd8b setup: set startup_info->have_repository more reliably
When setup_git_directory() is called, we set a flag in
startup_info to indicate we have a repository. But there are
a few other mechanisms by which we might set up a repo:

  1. When creating a new repository via init_db(), we
     transition from no-repo to being in a repo. We should
     tweak this flag at that moment.

  2. In enter_repo(), a stricter form of
     setup_git_directory() used by server-side programs, we
     check the repository format config. After doing so, we
     know we're in a repository, and can set the flag.

With these changes, library code can now reliably tell
whether we are in a repository and act accordingly. We'll
leave the "prefix" field as NULL, which is what happens when
setup_git_directory() finds there is no prefix.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-06 17:18:16 -08:00
Jeff King
46c3cd44d7 setup: make startup_info available everywhere
Commit a60645f (setup: remember whether repository was
found, 2010-08-05) introduced the startup_info structure,
which records some parts of the setup_git_directory()
process (notably, whether we actually found a repository or
not).

One of the uses of this data is for functions to behave
appropriately based on whether we are in a repo. But the
startup_info struct is just a pointer to storage provided by
the main program, and the only program that sets it up is
the git.c wrapper. Thus builtins have access to
startup_info, but externally linked programs do not.

Worse, library code which is accessible from both has to be
careful about accessing startup_info. This can be used to
trigger a die("BUG") via get_sha1():

	$ git fast-import <<-\EOF
	tag foo
	from HEAD:./whatever
	EOF

	fatal: BUG: startup_info struct is not initialized.

Obviously that's fairly nonsensical input to feed to
fast-import, but we should never hit a die("BUG"). And there
may be other ways to trigger it if other non-builtins
resolve sha1s.

So let's point the storage for startup_info to a static
variable in setup.c, making it available to all users of the
library code. We _could_ turn startup_info into a regular
extern struct, but doing so would mean tweaking all of the
existing use sites. So let's leave the pointer indirection
in place.  We can, however, drop any checks for NULL, as
they will always be false (and likewise, we can drop the
test covering this case, which was a rather artificial
situation using one of the test-* programs).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-06 17:17:37 -08:00
Junio C Hamano
11529ecec9 Merge branch 'jk/tighten-alloc'
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
  ...
2016-02-26 13:37:16 -08:00
Junio C Hamano
e6a6a768ca Merge branch 'nd/dwim-wildcards-as-pathspecs'
"git show 'HEAD:Foo[BAR]Baz'" did not interpret the argument as a
rev, i.e. the object named by the the pathname with wildcard
characters in a tree object.

* nd/dwim-wildcards-as-pathspecs:
  get_sha1: don't die() on bogus search strings
  check_filename: tighten dwim-wildcard ambiguity
  checkout: reorder check_filename conditional
2016-02-24 13:25:52 -08:00
Jeff King
3733e69464 use xmallocz to avoid size arithmetic
We frequently allocate strings as xmalloc(len + 1), where
the extra 1 is for the NUL terminator. This can be done more
simply with xmallocz, which also checks for integer
overflow.

There's no case where switching xmalloc(n+1) to xmallocz(n)
is wrong; the result is the same length, and malloc made no
guarantees about what was in the buffer anyway. But in some
cases, we can stop manually placing NUL at the end of the
allocated buffer. But that's only safe if it's clear that
the contents will always fill the buffer.

In each case where this patch does so, I manually examined
the control flow, and I tried to err on the side of caution.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-22 14:51:09 -08:00
Junio C Hamano
d0a1cbccab Merge branch 'nd/do-not-move-worktree-manually'
"git worktree" had a broken code that attempted to auto-fix
possible inconsistency that results from end-users moving a
worktree to different places without telling Git (the original
repository needs to maintain backpointers to its worktrees, but
"mv" run by end-users who are not familiar with that fact will
obviously not adjust them), which actually made things worse
when triggered.

* nd/do-not-move-worktree-manually:
  worktree: stop supporting moving worktrees manually
  worktree.c: fix indentation
2016-02-10 14:20:05 -08:00
Jeff King
df714f81a7 check_filename: tighten dwim-wildcard ambiguity
When specifying both revisions and pathnames, we allow
"<rev> -- <pathspec>" to be spelled without the "--" as long
as it is not ambiguous. The original logic was something
like:

  1. Resolve each item with get_sha1(). If successful,
     we know it can be a <rev>. Verify that it _isn't_ a
     filename, using verify_non_filename(), and complain of
     ambiguity otherwise.

  2. If get_sha1() didn't succeed, make sure that it _is_
     a file, using verify_filename(). If not, complain
     that it is neither a <rev> nor a <pathspec>.

Both verify_filename() and verify_non_filename() rely on
check_filename(), which definitely said "yes, this is a
file" or "no, it is not" using lstat().

Commit 28fcc0b (pathspec: avoid the need of "--" when
wildcard is used, 2015-05-02) introduced a convenience
feature: check_filename() will consider anything with
wildcard meta-characters as a possible filename, without
even checking the filesystem.

This works well for case 2. For such a wildcard, we would
previously have died and said "it is neither". Post-28fcc0b,
we assume it's a pathspec and proceed.

But it makes some instances of case 1 worse. We may have an
extended sha1 expression that contains meta-characters
(e.g., "HEAD^{/foo.*bar}"), and we now complain that it's
also a filename, due to the wildcard characters (even though
that wildcard would not match anything in the filesystem).

One solution would be to actually expand the pathname and
see if it matches anything on the filesystem. But that's
potentially expensive, and we do not have to be so rigorous
for this DWIM magic (if you want rigor, use "--").

Instead, we can just use different rules for cases 1 and 2.
When we know something is a rev, we will complain only if it
meets a much higher standard for "this is also a file";
namely that it actually exists in the filesystem. Case 2
remains the same: we use the looser "it could be a filename"
standard introduced by 28fcc0b.

We can accomplish this by pulling the wildcard logic out of
check_filename() and putting it into verify_filename(). Its
partner verify_non_filename() does not need a change, since
check_filename() goes back to implementing the "higher
standard".

Besides these two callers of check_filename(), there is one
other: git-checkout does a similar DWIM itself. It hits this
code path only after get_sha1() has returned failure, making
it case 2, which gets the special wildcard treatment.

Note that we drop the tests in t2019 in favor of a more
complete set in t6133. t2019 was not the right place for
them (it's about refname ambiguity, not dwim parsing
ambiguity), and the second test explicitly checked for the
opposite result of the case we are fixing here (which didn't
really make any sense; as shown by the test_must_fail in the
test, it would only serve to annoy people).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-10 13:53:20 -08:00
Jeff King
ffd036b128 clean: make is_git_repository a public function
We have always had is_git_directory(), for looking at a
specific directory to see if it contains a git repo. In
0179ca7 (clean: improve performance when removing lots of
directories, 2015-06-15), we added is_git_repository() which
checks for a non-bare repository by looking at its ".git"
entry.

However, the fix in 0179ca7 needs to be applied other
places, too. Let's make this new helper globally available.
We need to give it a better name, though, to avoid confusion
with is_git_directory(). This patch does that, documents
both functions with a comment to reduce confusion, and
removes the clean-specific references in the comments.

Based-on-a-patch-by: Andreas Krey <a.krey@gmx.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-25 11:41:53 -08:00
Nguyễn Thái Ngọc Duy
618244e160 worktree: stop supporting moving worktrees manually
The current update_linked_gitdir() has a bug that can create "gitdir"
file in non-multi-worktree setup. Worse, sometimes it can write relative
path to "gitdir" file, which will not work (e.g. "git worktree list"
will display the worktree's location incorrectly)

Instead of fixing this, we step back a bit. The original design was
probably not well thought out. For now, if the user manually moves a
worktree, they have to fix up "gitdir" file manually or the worktree
will get pruned.

Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-01-22 14:28:42 -08:00
Junio C Hamano
fa46579555 Merge branch 'jk/repository-extension'
Prepare for Git on-disk repository representation to undergo
backward incompatible changes by introducing a new repository
format version "1", with an extension mechanism.

* jk/repository-extension:
  introduce "preciousObjects" repository extension
  introduce "extensions" form of core.repositoryformatversion
2015-10-26 15:55:25 -07:00
Junio C Hamano
78891795df Merge branch 'jk/war-on-sprintf'
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
  ...
2015-10-20 15:24:01 -07:00
Jeff King
75faa45ae0 replace trivial malloc + sprintf / strcpy calls with xstrfmt
It's a common pattern to do:

  foo = xmalloc(strlen(one) + strlen(two) + 1 + 1);
  sprintf(foo, "%s %s", one, two);

(or possibly some variant with strcpy()s or a more
complicated length computation).  We can switch these to use
xstrfmt, which is shorter, involves less error-prone manual
computation, and removes many sprintf and strcpy calls which
make it harder to audit the code for real buffer overflows.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25 10:18:18 -07:00
Max Kirillov
11f9dd7191 path: implement common_dir handling in git_pathdup_submodule()
When submodule is a linked worktree, "git diff --submodule" and other
calls which directly access the submodule's object database do not correctly
calculate its path. Fix it by changing the git_pathdup_submodule() behavior,
to use either common or per-worktree directory.

Do it similarly as for parent repository, but ignore the GIT_COMMON_DIR
environment variable, because it would mean common directory for the parent
repository and does not make sense for submodule.

Also add test for functionality which uses this call.

Signed-off-by: Max Kirillov <max@max630.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-14 11:03:46 -07:00
Junio C Hamano
91d54694a4 Merge branch 'nd/fixup-linked-gitdir'
The code in "multiple-worktree" support that attempted to recover
from an inconsistent state updated an incorrect file.

* nd/fixup-linked-gitdir:
  setup: update the right file in multiple checkouts
2015-09-01 16:31:07 -07:00
Junio C Hamano
1f76a10b2d write_file(): drop caller-supplied LF from calls to create a one-liner file
All of the callsites covered by this change call write_file() or
write_file_gently() to create a one-liner file.  Drop the caller
supplied LF and let these callees to append it as necessary.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-25 12:49:19 -07:00
Nguyễn Thái Ngọc Duy
82fde87ff3 setup: update the right file in multiple checkouts
This code is introduced in 23af91d (prune: strategies for linked
checkouts - 2014-11-30), and it's supposed to implement this rule from
that commit's message:

 - linked checkouts are supposed to keep its location in $R/gitdir up
   to date. The use case is auto fixup after a manual checkout move.

Note the name, "$R/gitdir", not "$R/gitfile". Correct the path to be
updated accordingly.

While at there, make sure I/O errors are not silently dropped.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-25 09:39:08 -07:00
Junio C Hamano
12d6ce1dba write_file(): drop "fatal" parameter
All callers except three passed 1 for the "fatal" parameter to ask
this function to die upon error, but to a casual reader of the code,
it was not all obvious what that 1 meant.  Instead, split the
function into two based on a common write_file_v() that takes the
flag, introduce write_file_gently() as a new way to attempt creating
a file without dying on error, and make three callers to call it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-08-24 13:09:02 -07:00