Git for Windows ships with its own Perl interpreter, and insists on
using it, so it will most likely wreak havoc if PERL5LIB is set before
launching Git.
Let's just unset that environment variables when spawning processes.
To make this feature extensible (and overrideable), there is a new
config setting `core.unsetenvvars` that allows specifying a
comma-separated list of names to unset before spawning processes.
Reported by Gabriel Fuhrmann.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In the Git for Windows project, we have ample precendent for config
settings that apply to Windows, and to Windows only.
Let's formalize this concept by introducing a platform_core_config()
function that can be #define'd in a platform-specific manner.
This will allow us to contain platform-specific code better, as the
corresponding variables no longer need to be exported so that they can
be defined in environment.c and be set in config.c
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Coverity is a tool to analyze code statically, trying to find common (or
not so common) problems before they occur in production.
Coverity offers its services to Open Source software, and just like
upstream Git, Git for Windows applied and was granted the use.
While Coverity reports a lot of false positives due to Git's (ab-)use of
the FLEX_ARRAY feature (where it declares a 0-byte or 1-byte array at the
end of a struct, and then allocates a variable-length data structure
holding a variable-length string at the end, so that the struct as well as
the string can be released with a single free()), there were a few issues
reported that are true positives, and not all of them were resource leaks
in builtins (for which it is considered kind of okay to not release memory
just before exit() is called anyway).
This topic branch tries to address a couple of those issues.
Note: there are a couple more issues left, either because they are tricky
to resolve (in some cases, the custody of occasionally-allocated memory is
very unclear) or because it is unclear whether they are false positives
(due to the hard-to-reason-about nature of the code). It's a start,
though.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This topic branch allows us to specify absolute paths without the drive
prefix e.g. when cloning.
Example:
C:\Users\me> git clone https://github.com/git/git \upstream-git
This will clone into a new directory C:\upstream-git, in line with how
Windows interprets absolute paths.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
There is really no reason why we would need to hold onto the allocated
string longer than necessary.
Reported by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The buffer allocated by shorten_unambiguous_ref() needs to be released.
Discovered by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When the `name_rev()` function is asked to dereference the tip name, it
allocates memory. But when it turns out that another tip already
described the commit better than the current one, we forgot to release
the memory.
Pointed out by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The `guess_ref()` returns an allocated buffer of which `make_linked_ref()`
does not take custody (`alloc_ref()` makes a copy), therefore we need to
release the buffer afterwards.
Noticed via Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We free()d the `log` buffer when dwim_log() returned 1, but not when it
returned a larger value (which meant that it still allocated the buffer
but we simply ignored it).
While in the vicinity, make sure that the `reflogs` structure as well as
the `branch` variable are released properly, too.
Identified by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
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>
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>
This function did a whole lot of unnecessary work, such as reading in
four files just to figure out that, oh, hey, we do not need to look at
them after all because the HEAD is not detached.
Simplify the entire function to return early when possible, to read in
the files only when necessary, and to release the allocated memory
always (there was a leak, reported via Coverity, where we failed to
release the allocated strings if the HEAD is not detached).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This change addresses part of the NEEDSWORK comment above the code,
therefore the comment needs to be adjusted, too.
Discovered via Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
While POSIX states that it is okay to pass EOF to isspace() (and it seems
to be implied that EOF should *not* be treated as whitespace), and also to
pass EOF to ungetc() (which seems to be intended to fail without buffering
the character), it is much better to handle these cases explicitly. Not
only does it reduce head-scratching (and helps static analysis avoid
reporting false positives), it also lets us handle files containing
nothing but whitespace by erroring out.
Reported via Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This change plugs a couple of memory leaks and makes sure that the file
descriptor is closed in run_dir_diff().
Spotted by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When we fail to read, or parse, the file, we still want to close the file
descriptor and release the strbuf.
Reported via Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In case of errors, we really want the file descriptor to be closed.
Discovered by a Coverity scan.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
It would appear that we allocate (and forget to release) memory if the
patch ID is not even defined.
Reported by the Coverity tool.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When we could not convert the UTF-8 sequence into Unicode for writing to
the Console, we should not try to write an insanely-long sequence of
invalid wide characters (mistaking the negative return value for an
unsigned length).
Reported by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
To initialize the foreground color attributes of "plain text", our ANSI
emulation tries to infer them from the currently attached console while
running the is_console() function. This function first tries to detect any
console attached to stdout, then it is called with stderr.
If neither stdout nor stderr has any console attached, it does not
actually matter what we use for "plain text" attributes, as we never need
to output any text to any console in that case.
However, after working on stdout and stderr, is_console() is called with
stdin, and it still tries to initialize the "plain text" attributes if
they had not been initialized earlier. In this case, we cannot detect any
attributes, and we used an uninitialized value for them.
Naturally, Coverity complained about this use case because it could not
reason about the code deeply enough to figure out that we do not even use
those attributes in that case.
Let's just initialize the value to 0 in that case, both to avoid future
Coverity reports, and to help catch future regressions in case anybody
changes the order of the is_console() calls (which would make the text
black on black).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
In the (admittedly, concocted) case that PATH consists only of path
delimiters, we would leak the duplicated string.
Reported by Coverity.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This topic branch allows us to skip the gettext initialization
when the locale directory does not even exist.
This saves 150ms out of 210ms for a simply `git version` call on
Windows, and it most likely will help scripts that call out to
`git.exe` hundreds of times.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We newly handle isatty() by special-casing the stdin/stdout/stderr file
descriptors, caching the return value. However, we missed the case where
dup2() overrides the respective file descriptor.
That poses a problem e.g. where the `show` builtin asks for a pager very
early, the `setup_pager()` function sets the pager depending on the
return value of `isatty()` and then redirects stdout. Subsequently,
`cmd_log_init_finish()` calls `setup_pager()` *again*. What should
happen now is that `isatty()` reports that stdout is *not* a TTY and
consequently stdout should be left alone.
Let's override dup2() to handle this appropriately.
This fixes https://github.com/git-for-windows/git/issues/1077
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
This does not (yet) resolve the problem that Strg+T with multiple
selected lines fails to (un)stage them all, but it addresses one of
the reported Ctrl+T issues.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
We should not actually expect the first `attrib.exe` in the PATH to
be the one we are looking for. Or that it is in the PATH, for that
matter.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
A change between versions 2.4.1 and 2.6.0 of the MSYS2 runtime modified
how Cygwin's runtime (and hence Git for Windows' MSYS2 runtime
derivative) handles locales: d16a56306d (Consolidate wctomb/mbtowc calls
for POSIX-1.2008, 2016-07-20).
An unintended side-effect is that "cold-calling" into the POSIX
emulation will start with a locale based on the current code page,
something that Git for Windows is very ill-prepared for, as it expects
to be able to pass a command-line containing non-ASCII characters to the
shell without having those characters munged.
One symptom of this behavior: when `git clone` or `git fetch` shell out
to call `git-upload-pack` with a path that contains non-ASCII
characters, the shell tried to interpret the entire command-line
(including command-line parameters) as executable path, which obviously
must fail.
This fixes https://github.com/git-for-windows/git/issues/1036
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
The fix we introduced in Git for Windows will be made obsolete by a more
general fix that has been already accepted into upstream Git's `next`
branch.
But we still can introduce a regression test that verifies that this bug
will be caught very quickly, if reintroduced.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
When specifying an absolute path without a drive prefix, we convert that
path internally. Let's make sure that we handle that case properly, too
;-)
This fixes the command
git clone https://github.com/git-for-windows/git \G4W
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
On Windows, there are several categories of absolute paths. One such
category starts with a backslash and is implicitly relative to the
drive associated with the current working directory. Example:
c:
git clone https://github.com/git-for-windows/git \G4W
should clone into C:\G4W.
There is currently a problem with that, in that mingw_mktemp() does not
expect the _wmktemp() function to prefix the absolute path with the
drive prefix, and as a consequence, the resulting path does not fit into
the originally-passed string buffer. The symptom is a "Result too large"
error.
Reported by Juan Carlos Arevalo Baeza.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Teach do_write_index() to close the index.lock file
before getting the mtime and updating the istate.timestamp
fields.
On Windows, a file's mtime is not updated until the file is
closed. On Linux, the mtime is set after the last flush.
Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
The runtime of a simple `git.exe version` call on Windows is currently
dominated by the gettext setup, adding a whopping ~150ms to the ~210ms
total.
Given that this cost is added to each and every git.exe invocation goes
through common-main's invocation of git_setup_gettext(), and given that
scripts have to call git.exe dozens, if not hundreds, of times, this is
a substantial performance penalty.
This is particularly pointless when considering that Git for Windows
ships without localization (to keep the installer's size to a bearable
~34MB): all that time setting up gettext is for naught.
So let's be smart about it and skip setting up gettext if the locale
directory is not even present.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
