Code clean-up that also plugs memory leaks.
* jk/revision-remove-cmdline-pathspec:
pathspec doc: parse_pathspec does not maintain references to args
revision: replace "struct cmdline_pathspec" with argv_array
The scripts to drive TravisCI has been reorganized and then an
optimization to avoid spending cycles on a branch whose tip is
tagged has been implemented.
* ls/travis-scriptify:
travis-ci: fix "skip_branch_tip_with_tag()" string comparison
travis: dedent a few scripts that are indented overly deeply
travis-ci: skip a branch build if equal tag is present
travis-ci: move Travis CI code into dedicated scripts
"git gc" tries to avoid running two instances at the same time by
reading and writing pid/host from and to a lock file; it used to
use an incorrect fscanf() format when reading, which has been
corrected.
* aw/gc-lockfile-fscanf-fix:
gc: call fscanf() with %<len>s, not %<len>c, when reading hostname
A test to demonstrate "git mv" failing to adjust nested submodules
has been added.
* hv/mv-nested-submodules-test:
add test for bug in git-mv for recursive submodules
"git imap-send" has our own implementation of the protocol and also
can use more recent libCurl with the imap protocol support. Update
the latter so that it can use the credential subsystem, and then
make it the default option to use, so that we can eventually
deprecate and remove the former.
* nm/imap-send-with-curl:
imap-send: use curl by default when possible
imap_send: setup_curl: retreive credentials if not set in config file
imap-send: add wrapper to get server credentials if needed
imap-send: return with error if curl failed
The explanation of the cut-line in the commit log editor has been
slightly tweaked.
* ks/commit-do-not-touch-cut-line:
commit-template: change a message to be more intuitive
API error-proofing which happens to also squelch warnings from GCC.
* tg/refs-allowed-flags:
refs: strip out not allowed flags from ref_transaction_update
"git archive", especially when used with pathspec, stored an empty
directory in its output, even though Git itself never does so.
This has been fixed.
* rs/archive-excluded-directory:
archive: don't add empty directories to archives
Step #0 of a planned & larger series to make the in-core object
store per in-core repository object.
* jn/per-repo-object-store-fixes:
replace-objects: evaluate replacement refs without using the object store
push, fetch: error out for submodule entries not pointing to commits
pack: make packed_git_mru global a value instead of a pointer
Many codepaths did not diagnose write failures correctly when disks
go full, due to their misuse of write_in_full() helper function,
which have been corrected.
* jk/write-in-full-fix:
read_pack_header: handle signed/unsigned comparison in read result
config: flip return value of store_write_*()
notes-merge: use ssize_t for write_in_full() return value
pkt-line: check write_in_full() errors against "< 0"
convert less-trivial versions of "write_in_full() != len"
avoid "write_in_full(fd, buf, len) != len" pattern
get-tar-commit-id: check write_in_full() return against 0
config: avoid "write_in_full(fd, buf, len) < len" pattern
Doc update.
* kd/doc-for-each-ref:
doc/for-each-ref: explicitly specify option names
doc/for-each-ref: consistently use '=' to between argument names and values
Finishing touches to a topic already in 'master'.
* cc/subprocess-handshake-missing-capabilities:
subprocess: loudly die when subprocess asks for an unsupported capability
A hotfix to a topic already in 'master'.
* kw/write-index-reduce-alloc:
read-cache: fix index corruption with index v4
Add t/helper/test-write-cache to .gitignore
A handful of tests to demonstrates a recursive implementation of
"name-rev" hurts.
* mg/name-rev-tests-with-short-stack:
t6120: test describe and name-rev with deep repos
t6120: clean up state after breaking repo
t6120: test name-rev --all and --stdin
t7004: move limited stack prereq to test-lib
Currently, when parse_options() produces a help message it always emits
a blank line after the usage text to separate it from the options text.
If the option spec does not define any switches, or only defines hidden
switches that will not be displayed, then the help text will end up with
two trailing blank lines instead of one. Let's defer emitting the blank
line between the usage text and the options text until it is clear that
the options section will not be empty.
Fixes t1502.5, t1502.6.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When commit 54e6dc7 added translation support to parse-options, an
fprintf was mistakenly replaced by a call to putchar(). Let's use fputc
instead.
Fixes t0040.11, t0040.12, t0040.33, and t1502.8.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When the option spec contains no switches or only hidden switches,
parse_options will emit an extra blank line at the end of help output so
that the help text will end in two blank lines instead of one.
When parse_options produces internal help output after an error has
occurred it will emit blank lines within the usage string to stdout
instead of stderr.
Update t/helper/test-parse-options.c to have a description body in the
usage string to exercise this second bug and mark tests as failing in
t0040.
Add tests to t1502 to demonstrate both of these problems.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The description of `-Xours` merge option has a parenthetical note
that tells the readers that it is very different from `-s ours`,
which is correct, but the description of `-Xtheirs` that follows it
carelessly says "this is the opposite of `ours`", giving a false
impression that the readers also need to be warned that it is very
different from `-s theirs`, which in reality does not even exist.
Clarify it a bit to avoid misleading readers.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
With more commands (that potentially change a submodule) paying attention
to submodules as well as the recent discussion[1] on
submodule.<name>.update, let's spell out that submodule.<name>.update
is strictly to be used for configuring the "submodule update" command
and not to be obeyed by other commands.
These other commands usually have a strict meaning of what they should
do (i.e. checkout, reset, rebase, merge) as well as have their name
overlapping with the modes possible for submodule.<name>.update.
[1] https://public-inbox.org/git/4283F0B0-BC1C-4ED1-8126-7E512D84484B@gmail.com/
submodule.<name>.update was set to "none", triggering unexpected
behavior as the submodule was thought to never be touched.
However a newer version of Git taught 'git pull --rebase' to also
populate and rebase submodules if they were active.
The newer options such as submodule.active and command specific
flags would not have triggered unexpected behavior.
Reported-by: Lars Schneider <larsxschneider@gmail.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Decode =XY in quoted-printable segments only if X and Y are hexadecimal
digits, otherwise just copy them. That's at least better than
interpreting negative results from hexval() as a character.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This allows us to get rid of some write-only variables, among them seven
SHA1 buffers.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This allows us to get rid of two write-only variables, one of them
being a SHA1 buffer.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Allow callers of refs_resolve_ref_unsafe() to pass NULL if they don't
need the resolved hash value. We already allow the same for the flags
parameter. This new leniency is inherited by the various wrappers like
resolve_ref_unsafe().
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Current HTML header regexp doesn't match headers without attributes.
So it fails to match <h1>...</h1>, while <h1 class="smth">...</h1> matches.
Make attributes optional to fix this. The regexp is still far from
perfect, but now it at least handles the common case.
Signed-off-by: Ilya Kantor <iliakan@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The git-read-tree manpage has a table that is meant to
be shown with its spacing exactly as it is in the source. We
mark it as a "literal paragraph" by indenting each line by
at least one space. This renders OK with asciidoc for both
the HTML and manpage versions.
But there are two problems when we render it with
asciidoctor.
The first is that some lines mix tabs and spaces. Even if
asciidoctor is correctly configured for 8-space tabs, it
seems to handle this case differently, soaking up some of
the initial literal-paragraph spaces and mis-aligning the
table text.
The second problem is that the table uses blank lines to
group rows. But as blank lines separate paragraphs in
asciidoc, this actually means that each chunk of the table
is rendered in its own pre-formatted <div> block. This
happens even with vanilla asciidoc, but there's no visible
result because the literal paragraphs aren't styled in any
special way. But with asciidoctor (or at least the styles
used on git-scm.com), literal paragraphs are styled with a
different background. This breaks the table into a visually
distracting sequence of chunks.
We can fix both by adding a literal-paragraph block
delimiter. That turns the whole table into a single block
(for both implementations) and causes asciidoctor to render
the indentation as it is in the source.
Reported-at: https://github.com/git/git-scm.com/issues/1023
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of setting the fields of rev->pending to 0/NULL, thereby leaking
memory, call `object_array_clear(&rev->pending)`.
In pack-bitmap.c, we make copies of those fields as `pending_nr` and
`pending_e`. We never update the aliases and the original fields never
change, so the aliases are not really needed and just make it harder
than necessary to understand the code. While we're here, remove the
aliases to make the code easier to follow.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a couple of places, we pop objects off an object array `foo` by
decreasing `foo.nr`. We access `foo.nr` in many places, but most if not
all other times we do so read-only, e.g., as we iterate over the array.
But when we change `foo.nr` behind the array's back, it feels a bit
nasty and looks like it might leak memory.
Leaks happen if the popped element has an allocated `name` or `path`.
At the moment, that is not the case. Still, 1) the object array might
gain more fields that want to be freed, 2) a code path where we pop
might start using names or paths, 3) one of these code paths might be
copied to somewhere where we do, and 4) using a dedicated function for
popping is conceptually cleaner.
Introduce and use `object_array_pop()` instead. Release memory in the
new function. Document that popping an object leaves the associated
elements in limbo.
The converted places were identified by grepping for "\.nr\>" and
looking for "--".
Make the new function return NULL on an empty array. This is consistent
with `pop_commit()` and allows the following:
while ((o = object_array_pop(&foo)) != NULL) {
// do something
}
But as noted above, we don't need to go out of our way to avoid reading
`foo.nr`. This is probably more readable:
while (foo.nr) {
... o = object_array_pop(&foo);
// do something
}
The name of `object_array_pop()` does not quite align with
`add_object_array()`. That is unfortunate. On the other hand, it matches
`object_array_clear()`. Arguably it's `add_...` that is the odd one out,
since it reads like it's used to "add" an "object array". For that
reason, side with `object_array_clear()`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Instead of freeing `foo.objects` for an object array `foo` (sometimes
conditionally), call `object_array_clear(&foo)`. This means we don't
poke as much into the implementation, which is already a good thing, but
also that we release the individual entries as well, thereby fixing at
least one memory-leak (in diff-lib.c).
If someone is holding on to a pointer to an element's `name` or `path`,
that is now a dangling pointer, i.e., we'd be turning an unpleasant
situation into an outright bug. To the best of my understanding no such
long-term pointers are being taken.
The way we handle `study` in builting/reflog.c still looks like it might
leak. That will be addressed in the next commit.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Setting `leak_pending = 1` tells `prepare_revision_walk()` not to
release the `pending` array, and makes that the caller's responsibility.
See 4a43d374f (revision: add leak_pending flag, 2011-10-01) and
353f5657a (bisect: use leak_pending flag, 2011-10-01).
Commit 1da1e07c8 (clean up name allocation in prepare_revision_walk,
2014-10-15) fixed a memory leak in `prepare_revision_walk()` by
switching from `free()` to `object_array_clear()`. However, where we use
the `leak_pending`-mechanism, we're still only calling `free()`.
Use `object_array_clear()` instead. Copy some helpful comments from
353f5657a to the other callers that we update to clarify the memory
responsibilities, and to highlight that the commits are not affected
when we clear the array -- it is indeed correct to both tidy up the
commit flags and clear the object array.
Document `leak_pending` in revision.h to help future users get this
right.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
We don't free the temporary scratch space we use with
`remove_redundant()`. Free it similar to how we do it in
`get_merge_bases_many_0()`.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Release `pathspec` and the string list `partial`.
When we clear the string list, make sure we do not free the `util`
pointers. That would result in double-freeing, since we set them up as
`item->util = item` in `list_paths()`.
Initialize the string list early, so that we can always release it. That
introduces some unnecessary overhead in various code paths, but means
there is one and only one way out of the function. If we ever accumulate
more things we need to free, it should be straightforward to do so.
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When creating a new branch B by copying the branch A that happens to
be the current branch, it also updates HEAD to point at the new
branch. It probably was made this way because "git branch -c A B"
piggybacked its implementation on "git branch -m A B",
This does not match the usual expectation. If I were sitting on a
blue chair, and somebody comes and repaints it to red, I would
accept ending up sitting on a chair that is now red (I am also OK to
stand, instead, as there no longer is my favourite blue chair). But
if somebody creates a new red chair, modelling it after the blue
chair I am sitting on, I do not expect to be booted off of the blue
chair and ending up on sitting on the new red one.
Let's fix this before it hits 'next'. Those who want to create a
new branch and switch to it can do "git checkout B" after doing a
"git branch -c B", and if that operation is so useful and deserves a
short-hand way to do so, perhaps extend "git checkout -b B" to copy
configurations while creating the new branch B.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano