Commit graph

102 commits

Author SHA1 Message Date
Junio C Hamano
bedd3b4b7b Merge branch 'nd/large-blobs'
Teach a few codepaths to punt (instead of dying) when large blobs
that would not fit in core are involved in the operation.

* nd/large-blobs:
  diff: shortcut for diff'ing two binary SHA-1 objects
  diff --stat: mark any file larger than core.bigfilethreshold binary
  diff.c: allow to pass more flags to diff_populate_filespec
  sha1_file.c: do not die failing to malloc in unpack_compressed_entry
  wrapper.c: introduce gentle xmallocz that does not die()
2014-09-11 10:33:33 -07:00
Steffen Prohaska
9927d9627f memory_limit: use git_env_ulong() to parse GIT_ALLOC_LIMIT
GIT_ALLOC_LIMIT limits xmalloc()'s size, which is of type size_t.
Better use git_env_ulong() to parse the environment variable, so
that the postfixes 'k', 'm', and 'g' can be used; and use size_t to
store the limit for consistency.  The change to size_t has no direct
practical impact, because the environment variable is only meant to
be used for our own tests, and we use it to test small sizes.

The cast of size in the call to die() is changed to uintmax_t to
match the format string PRIuMAX.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-28 10:25:04 -07:00
René Scharfe
aa14e980ff wrapper: add xgetcwd()
Add the helper function xgetcwd(), which returns the current directory
or dies.  The returned string has to be free()d after use.

Helped-by: Duy Nguyen <pclouds@gmail.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-26 11:06:05 -07:00
Nguyễn Thái Ngọc Duy
f8bb1d9431 wrapper.c: introduce gentle xmallocz that does not die()
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-18 10:15:08 -07:00
Yiannis Marangos
426ddeead6 read-cache.c: verify index file before we opportunistically update it
Before we proceed to opportunistically update the index (often done
by an otherwise read-only operation like "git status" and "git diff"
that internally refreshes the index), we must verify that the
current index file is the same as the one that we read earlier
before we took the lock on it, in order to avoid a possible race.

In the example below git-status does "opportunistic update" and
git-rebase updates the index, but the race can happen in general.

  1. process A calls git-rebase (or does anything that uses the index)

  2. process A applies 1st commit

  3. process B calls git-status (or does anything that updates the index)

  4. process B reads index

  5. process A applies 2nd commit

  6. process B takes the lock, then overwrites process A's changes.

  7. process A applies 3rd commit

As an end result the 3rd commit will have a revert of the 2nd commit.
When process B takes the lock, it needs to make sure that the index
hasn't changed since step 4.

Signed-off-by: Yiannis Marangos <yiannis.marangos@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-10 12:27:58 -07:00
Yiannis Marangos
9aa91af036 wrapper.c: add xpread() similar to xread()
It is a common mistake to call read(2)/pread(2) and forget to
anticipate that they may return error with EAGAIN/EINTR when the
system call is interrupted.

We have xread() helper to relieve callers of read(2) from having to
worry about it; add xpread() helper to do the same for pread(2).

Update the caller in the builtin/index-pack.c and the mmap emulation
in compat/.

Signed-off-by: Yiannis Marangos <yiannis.marangos@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-10 12:18:55 -07:00
Masanari Iida
382d20e3eb typofixes: fix misspelt comments
Signed-off-by: Masanari Iida <standby24x7@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-11-12 09:24:27 -08:00
Ramsay Jones
ec145c9c2e wrapper.c: only define gitmkstemps if needed
When the NO_MKSTEMPS build variable is not set, the gitmkstemps
function is dead code.  Use a preprocessor conditional to only include
the definition when needed.

Noticed by sparse.  ("'gitmkstemps' was not declared. Should it be
static?")

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
2013-10-14 16:16:00 -07:00
Steffen Prohaska
0b6806b9e4 xread, xwrite: limit size of IO to 8MB
Checking out 2GB or more through an external filter (see test) fails
on Mac OS X 10.8.4 (12E55) for a 64-bit executable with:

    error: read from external filter cat failed
    error: cannot feed the input to external filter cat
    error: cat died of signal 13
    error: external filter cat failed 141
    error: external filter cat failed

The reason is that read() immediately returns with EINVAL when asked
to read more than 2GB.  According to POSIX [1], if the value of
nbyte passed to read() is greater than SSIZE_MAX, the result is
implementation-defined.  The write function has the same restriction
[2].  Since OS X still supports running 32-bit executables, the
32-bit limit (SSIZE_MAX = INT_MAX = 2GB - 1) seems to be also
imposed on 64-bit executables under certain conditions.  For write,
the problem has been addressed earlier [6c642a].

Address the problem for read() and write() differently, by limiting
size of IO chunks unconditionally on all platforms in xread() and
xwrite().  Large chunks only cause problems, like causing latencies
when killing the process, even if OS X was not buggy.  Doing IO in
reasonably sized smaller chunks should have no negative impact on
performance.

The compat wrapper clipped_write() introduced earlier [6c642a] is
not needed anymore.  It will be reverted in a separate commit.  The
new test catches read and write problems.

Note that 'git add' exits with 0 even if it prints filtering errors
to stderr.  The test, therefore, checks stderr.  'git add' should
probably be changed (sometime in another commit) to exit with
nonzero if filtering fails.  The test could then be changed to use
test_must_fail.

Thanks to the following people for suggestions and testing:

    Johannes Sixt <j6t@kdbg.org>
    John Keeping <john@keeping.me.uk>
    Jonathan Nieder <jrnieder@gmail.com>
    Kyle J. McKay <mackyle@gmail.com>
    Linus Torvalds <torvalds@linux-foundation.org>
    Torsten Bögershausen <tboegi@web.de>

[1] http://pubs.opengroup.org/onlinepubs/009695399/functions/read.html
[2] http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

[6c642a] commit 6c642a8786
    compate/clipped-write.c: large write(2) fails on Mac OS X/XNU

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-08-20 11:10:59 -07:00
Junio C Hamano
1d1934caf1 Merge branch 'tr/fd-gotcha-fixes'
Two places we did not check return value (expected to be a file
descriptor) correctly.

* tr/fd-gotcha-fixes:
  run-command: dup_devnull(): guard against syscalls failing
  git_mkstemps: correctly test return value of open()
2013-07-22 11:23:13 -07:00
Dale R. Worley
a2cb86c152 git_mkstemps: correctly test return value of open()
open() returns -1 on failure, and indeed 0 is a possible success value
if the user closed stdin in our process.  Fix the test.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-07-12 10:30:08 -07:00
Jonathan Nieder
4698c8feb1 config: allow inaccessible configuration under $HOME
The changes v1.7.12.1~2^2~4 (config: warn on inaccessible files,
2012-08-21) and v1.8.1.1~22^2~2 (config: treat user and xdg config
permission problems as errors, 2012-10-13) were intended to prevent
important configuration (think "[transfer] fsckobjects") from being
ignored when the configuration is unintentionally unreadable (for
example with EIO on a flaky filesystem, or with ENOMEM due to a DoS
attack).  Usually ~/.gitconfig and ~/.config/git are readable by the
current user, and if they aren't then it would be easy to fix those
permissions, so the damage from adding this check should have been
minimal.

Unfortunately the access() check often trips when git is being run as
a server.  A daemon (such as inetd or git-daemon) starts as "root",
creates a listening socket, and then drops privileges, meaning that
when git commands are invoked they cannot access $HOME and die with

 fatal: unable to access '/root/.config/git/config': Permission denied

Any patch to fix this would have one of three problems:

  1. We annoy sysadmins who need to take an extra step to handle HOME
     when dropping privileges (the current behavior, or any other
     proposal that they have to opt into).

  2. We annoy sysadmins who want to set HOME when dropping privileges,
     either by making what they want to do impossible, or making them
     set an extra variable or option to accomplish what used to work
     (e.g., a patch to git-daemon to set HOME when --user is passed).

  3. We loosen the check, so some cases which might be noteworthy are
     not caught.

This patch is of type (3).

Treat user and xdg configuration that are inaccessible due to
permissions (EACCES) as though no user configuration was provided at
all.

An alternative method would be to check if $HOME is readable, but that
would not help in cases where the user who dropped privileges had a
globally readable HOME with only .config or .gitconfig being private.

This does not change the behavior when /etc/gitconfig or .git/config
is unreadable (since those are more serious configuration errors),
nor when ~/.gitconfig or ~/.config/git is unreadable due to problems
other than permissions.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Improved-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-04-15 07:26:50 -07:00
Junio C Hamano
e6f1550aa5 Merge branch 'jn/warn-on-inaccessible-loosen' into maint
When attempting to read the XDG-style $HOME/.config/git/config and
finding that $HOME/.config/git is a file, we gave a wrong error
message, instead of treating the case as "a custom config file does
not exist there" and moving on.

* jn/warn-on-inaccessible-loosen:
  config: exit on error accessing any config file
  doc: advertise GIT_CONFIG_NOSYSTEM
  config: treat user and xdg config permission problems as errors
  config, gitignore: failure to access with ENOTDIR is ok
2013-01-11 16:47:07 -08:00
Junio C Hamano
4f43e9726a Merge branch 'jn/warn-on-inaccessible-loosen'
Deal with a situation where .config/git is a file and we notice
.config/git/config is not readable due to ENOTDIR, not ENOENT.

* jn/warn-on-inaccessible-loosen:
  config: exit on error accessing any config file
  doc: advertise GIT_CONFIG_NOSYSTEM
  config: treat user and xdg config permission problems as errors
  config, gitignore: failure to access with ENOTDIR is ok
2013-01-06 22:11:16 -08:00
Junio C Hamano
f7be59b477 xmkstemp(): avoid showing truncated template more carefully
Some implementations of xmkstemp() leaves the given in/out buffer
truncated when they return with failure.

6cf6bb3 (Improve error messages when temporary file creation fails,
2010-12-18) attempted to show the real filename we tried to create
(but failed), and if that is not available due to such truncation,
to show the original template that was given by the caller.

But it failed to take into account that the given template could
have "directory/" in front, in which case the truncation point may
not be template[0] but somewhere else.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-12-18 13:02:33 -08:00
Jonathan Nieder
96b9e0e313 config: treat user and xdg config permission problems as errors
Git reads multiple configuration files: settings come first from the
system config file (typically /etc/gitconfig), then the xdg config
file (typically ~/.config/git/config), then the user's dotfile
(~/.gitconfig), then the repository configuration (.git/config).

Git has always used access(2) to decide whether to use each file; as
an unfortunate side effect, that means that if one of these files is
unreadable (e.g., EPERM or EIO), git skips it.  So if I use
~/.gitconfig to override some settings but make a mistake and give it
the wrong permissions then I am subject to the settings the sysadmin
chose for /etc/gitconfig.

Better to error out and ask the user to correct the problem.

This only affects the user and xdg config files, since the user
presumably has enough access to fix their permissions.  If the system
config file is unreadable, the best we can do is to warn about it so
the user knows to notify someone and get on with work in the meantime.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-10-13 21:59:16 -07:00
Jonathan Nieder
e5c52c9898 config, gitignore: failure to access with ENOTDIR is ok
The access_or_warn() function is used to check for optional
configuration files like .gitconfig and .gitignore and warn when they
are not accessible due to a configuration issue (e.g., bad
permissions).  It is not supposed to complain when a file is simply
missing.

Noticed on a system where ~/.config/git was a file --- when the new
XDG_CONFIG_HOME support looks for ~/.config/git/config it should
ignore ~/.config/git instead of printing irritating warnings:

 $ git status -s
 warning: unable to access '/home/jrn/.config/git/config': Not a directory
 warning: unable to access '/home/jrn/.config/git/config': Not a directory
 warning: unable to access '/home/jrn/.config/git/config': Not a directory
 warning: unable to access '/home/jrn/.config/git/config': Not a directory

Compare v1.7.12.1~2^2 (attr:failure to open a .gitattributes file
is OK with ENOTDIR, 2012-09-13).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-10-13 21:59:13 -07:00
Junio C Hamano
55b38a48e2 warn_on_inaccessible(): a helper to warn on inaccessible paths
The previous series introduced warnings to multiple places, but it
could become tiring to see the warning on the same path over and
over again during a single run of Git.  Making just one function
responsible for issuing this warning, we could later choose to keep
track of which paths we issued a warning (it would involve a hash
table of paths after running them through real_path() or something)
in order to reduce noise.

Right now we do not know if the noise reduction is necessary, but it
still would be a good code reduction/sharing anyway.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-21 14:52:07 -07:00
Jeff King
ba8bd8300a config: warn on inaccessible files
Before reading a config file, we check "!access(path, R_OK)"
to make sure that the file exists and is readable. If it's
not, then we silently ignore it.

For the case of ENOENT, this is fine, as the presence of the
file is optional. For other cases, though, it may indicate a
configuration error (e.g., not having permissions to read
the file). Let's print a warning in these cases to let the
user know.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-08-21 14:46:11 -07:00
Jeff King
2f70587502 ident: report passwd errors with a more friendly message
When getpwuid fails, we give a cute but cryptic message.
While it makes sense if you know that getpwuid or identity
functions are being called, this code is triggered behind
the scenes by quite a few git commands these days (e.g.,
receive-pack on a remote server might use it for a reflog;
the current message is hard to distinguish from an
authentication error).  Let's switch to something that gives
a little more context.

While we're at it, we can factor out all of the
cut-and-pastes of the "you don't exist" message into a
wrapper function. Rather than provide xgetpwuid, let's make
it even more specific to just getting the passwd entry for
the current uid. That's the only way we use getpwuid anyway,
and it lets us make an even more specific error message.

The current message also fails to mention errno. While the
usual cause for getpwuid failing is that the user does not
exist, mentioning errno makes it easier to diagnose these
problems.  Note that POSIX specifies that errno remain
untouched if the passwd entry does not exist (but will be
set on actual errors), whereas some systems will return
ENOENT or similar for a missing entry. We handle both cases
in our wrapper.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-05-22 09:08:20 -07:00
Nguyễn Thái Ngọc Duy
d41489a642 Add more large blob test cases
New test cases list commands that should work when memory is
limited. All memory allocation functions (*) learn to reject any
allocation larger than $GIT_ALLOC_LIMIT if set.

(*) Not exactly all. Some places do not use x* functions, but
malloc/calloc directly, notably diff-delta. These code path should
never be run on large blobs.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-03-07 09:07:37 -08:00
Jeff King
56d7c27af1 read_in_full: always report errors
The read_in_full function repeatedly calls read() to fill a
buffer. If the first read() returns an error, we notify the
caller by returning the error. However, if we read some data
and then get an error on a subsequent read, we simply return
the amount of data that we did read, and the caller is
unaware of the error.

This makes the tradeoff that seeing the partial data is more
important than the fact that an error occurred. In practice,
this is generally not the case; we care more if an error
occurred, and should throw away any partial data.

I audited the current callers. In most cases, this will make
no difference at all, as they do:

  if (read_in_full(fd, buf, size) != size)
	  error("short read");

However, it will help in a few cases:

  1. In sha1_file.c:index_stream, we would fail to notice
     errors in the incoming stream.

  2. When reading symbolic refs in resolve_ref, we would
     fail to notice errors and potentially use a truncated
     ref name.

  3. In various places, we will get much better error
     messages. For example, callers of safe_read would
     erroneously print "the remote end hung up unexpectedly"
     instead of showing the read error.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-05-26 13:54:18 -07:00
Carlos Martín Nieto
e2a57aac8a Name make_*_path functions more accurately
Rename the make_*_path functions so it's clearer what they do, in
particlar make clear what the differnce between make_absolute_path and
make_nonrelative_path is by renaming them real_path and absolute_path
respectively. make_relative_path has an understandable name and is
renamed to relative_path to maintain the name convention.

The function calls have been replaced 1-to-1 in their usage.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-03-17 16:08:30 -07:00
Junio C Hamano
b308bf1863 Merge branch 'maint'
* maint:
  compat: helper for detecting unsigned overflow
2011-02-10 14:45:55 -08:00
Jonathan Nieder
1368f65002 compat: helper for detecting unsigned overflow
The idiom (a + b < a) works fine for detecting that an unsigned
integer has overflowed, but a more explicit

	unsigned_add_overflows(a, b)

might be easier to read.

Define such a macro, expanding roughly to ((a) < UINT_MAX - (b)).
Because the expansion uses each argument only once outside of sizeof()
expressions, it is safe to use with arguments that have side effects.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-02-10 13:47:56 -08:00
Arnout Engelen
6cf6bb3e47 Improve error messages when temporary file creation fails
Before, when creating a temporary file failed, a generic 'Unable to create
temporary file' message was printed. In some cases this could lead to
confusion as to which directory should be checked for correct permissions etc.

This patch adds the template for the temporary filename to the error message,
converting it to an absolute path if needed. A test verifies that the template
is indeed printed when pointing to a nonexistent or unwritable directory.

A copy of the original template is made in case mkstemp clears the template.

Signed-off-by: Arnout Engelen <arnouten@bzzt.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-12-21 19:51:17 -08:00
Junio C Hamano
00b0d7f77b set_try_to_free_routine(NULL) means "do nothing special"
This way, the next caller that wants to disable our memory reclamation
machinery does not have to define its own do_nothing() stub.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-12-21 09:25:14 -08:00
Jonathan Nieder
e050029385 Remove pack file handling dependency from wrapper.o
As v1.7.0-rc0~43 (slim down "git show-index", 2010-01-21) explains,
use of xmalloc() brings in a dependency on zlib, the sha1 lib, and the
rest of git's object file access machinery via try_to_free_pack_memory.
That is overkill when xmalloc is just being used as a convenience
wrapper to exit when no memory is available.

So defer setting try_to_free_pack_memory as try_to_free_routine until
the first packfile is opened in add_packed_git().

After this change, a simple program using xmalloc() and no other
functions will not pull in any code from libgit.a aside from wrapper.o
and usage.o.

Improved-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-10 11:11:07 -08:00
Jonathan Nieder
b0613ce0f9 wrapper: give zlib wrappers their own translation unit
Programs using xmalloc() but not git_inflate() require -lz on the
linker command line because git_inflate() is in the same translation
unit as xmalloc().

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-10 11:07:51 -08:00
Jonathan Nieder
33f239365c path helpers: move git_mkstemp* to wrapper.c
git_mkstemp_mode and related functions do not require access to
specialized git machinery, unlike some other functions from
path.c (like set_shared_perm()).  Move them to wrapper.c where
the wrapper xmkstemp_mode is defined.

This eliminates a dependency of wrapper.o on environment.o via
path.o.  With typical linkers (e.g., gcc), that dependency makes
programs that use functions from wrapper.o and not environment.o
or path.o larger than they need to be.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-10 11:04:26 -08:00
Jonathan Nieder
463db9b104 wrapper: move odb_* to environment.c
The odb_mkstemp and odb_pack_keep functions open files under the
$GIT_OBJECT_DIRECTORY directory.  This requires access to the git
configuration which very simple programs do not need.

Move these functions to environment.o, closer to their dependencies.
This should make it easier for programs to link to wrapper.o without
linking to environment.o.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-10 11:03:38 -08:00
Jonathan Nieder
58ecbd5ede wrapper: move xmmap() to sha1_file.c
wrapper.o depends on sha1_file.o for a number of reasons.  One is
release_pack_memory().

xmmap function calls mmap, discarding unused pack windows when
necessary to relieve memory pressure.  Simple git programs using
wrapper.o as a friendly libc do not need this functionality.
So move xmmap to sha1_file.o, where release_pack_memory() is.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-11-10 11:03:13 -08:00
Matthieu Moy
8bd9fd5001 xmalloc: include size in the failure message
Out-of-memory errors can either be actual lack of memory, or bugs (like
code trying to call xmalloc(-1) by mistake). A little more information
may help tracking bugs reported by users.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-20 09:27:38 -07:00
Junio C Hamano
44e08b003d Merge branch 'js/try-to-free-stackable'
* js/try-to-free-stackable:
  Do not call release_pack_memory in malloc wrappers when GIT_TRACE is used
  Have set_try_to_free_routine return the previous routine
2010-06-13 11:21:21 -07:00
Junio C Hamano
ea5f75a64a Merge branch 'np/malloc-threading'
* np/malloc-threading:
  Thread-safe xmalloc and xrealloc needs a recursive mutex
  Make xmalloc and xrealloc thread-safe
2010-05-21 04:02:16 -07:00
Johannes Sixt
851c34b04e Have set_try_to_free_routine return the previous routine
This effectively requires from the callers of set_try_to_free_routine to
treat the try-to-free-routines as a stack.

We will need this for the next patch where the only current caller cannot
depend on that the previously set routine was the default routine.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-05-08 22:27:54 -07:00
Peter Collingbourne
80d706afed Introduce remove_or_warn function
This patch introduces the remove_or_warn function which is a
generalised version of the {unlink,rmdir}_or_warn functions.  It takes
an additional parameter indicating the mode of the file to be removed.

The patch also modifies certain functions to use remove_or_warn
where appropriate, and adds a test case for a bug fixed by the use
of remove_or_warn.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-28 09:53:04 -07:00
Peter Collingbourne
d1723296af Implement the rmdir_or_warn function
This patch implements an rmdir_or_warn function (like unlink_or_warn
but for directories) that uses the generalised warning code in
warn_if_unremovable.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-28 09:53:01 -07:00
Peter Collingbourne
10e13ec8ed Generalise the unlink_or_warn function
This patch moves the warning code of the unlink_or_warn function into
a separate function named warn_if_unremovable so that it may be reused.

Signed-off-by: Peter Collingbourne <peter@pcc.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-28 09:52:59 -07:00
Nicolas Pitre
a9a746364b Make xmalloc and xrealloc thread-safe
By providing a hook for the routine responsible for trying to free some
memory on malloc failure, we can ensure that the  called routine is
protected by the appropriate locks when threads are in play.

The obvious offender here was pack-objects which was calling xmalloc()
within threads while release_pack_memory() is not thread safe.

Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-03-24 14:15:09 -07:00
Matthieu Moy
f80c7ae8fe Use git_mkstemp_mode and xmkstemp_mode in odb_mkstemp, not chmod later.
We used to create 0600 files, and then use chmod to set the group and
other permission bits to the umask. This usually has the same effect
as a normal file creation with a umask.

But in the presence of ACLs, the group permission plays the role of
the ACL mask: the "g" bits of newly created files are chosen according
to default ACL mask of the directory, not according to the umask, and
doing a chmod() on these "g" bits affect the ACL's mask instead of
actual group permission.

In other words, creating files with 0600 and then doing a chmod to the
umask creates files which are unreadable by users allowed in the
default ACL. To create the files without breaking ACLs, we let the
umask do it's job at the file's creation time, and get rid of the
later chmod.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-22 15:24:46 -08:00
Matthieu Moy
b862b61c03 git_mkstemp_mode, xmkstemp_mode: variants of gitmkstemps with mode argument.
gitmkstemps emulates the behavior of mkstemps, which is usually used
to create files in a shared directory like /tmp/, hence, it creates
files with permission 0600.

Add git_mkstemps_mode() that allows us to specify the desired mode, and
make git_mkstemps() a wrapper that always uses 0600 to call it. Later we
will use git_mkstemps_mode() when creating pack files.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-22 15:24:45 -08:00
Ilari Liusvaara
5bf9219d01 Add xmallocz()
Add routine for allocating NUL-terminated memory block without risking
integer overflow in addition of +1 for NUL byte.

[jc: with suggestion from Bill Lear]

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-01-26 12:57:53 -08:00
Thomas Rast
d824cbba02 Convert existing die(..., strerror(errno)) to die_errno()
Change calls to die(..., strerror(errno)) to use the new die_errno().

In the process, also make slight style adjustments: at least state
_something_ about the function that failed (instead of just printing
the pathname), and put paths in single quotes.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-06-27 11:14:53 -07:00
Alex Riesen
fc71db39e0 Introduce an unlink(2) wrapper which gives warning if unlink failed
This seem to be a very common pattern in the current code.

The function prints a generic removal failure message, the file name
which failed and readable errno presentation. The function preserves
errno and always returns the value unlink(2) returned, but prints
no message for ENOENT, as it was the most often filtered out in the
code calling unlink. Besides, removing a file is anyway the purpose of
calling unlink.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-04-29 18:37:41 -07:00
Junio C Hamano
f474c52661 Merge branch 'jc/maint-1.6.0-pack-directory'
* jc/maint-1.6.0-pack-directory:
  Fix odb_mkstemp() on AIX
2009-02-28 16:31:02 -08:00
Mike Ralphson
2c626e5fa8 Fix odb_mkstemp() on AIX
The AIX mkstemp() modifies its template parameter to an empty string if
the call fails.  The existing code had already recomputed the template,
but too late to be good.

See also 6ff6af62, which fixed this problem in a different spot.

Signed-off-by: Mike Ralphson <mike@abacus.co.uk>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-02-26 08:40:27 -08:00
Junio C Hamano
bb0cebd7d0 Merge branch 'jc/maint-1.6.0-pack-directory'
* jc/maint-1.6.0-pack-directory:
  Make sure objects/pack exists before creating a new pack
2009-02-25 14:50:05 -08:00
Junio C Hamano
6e180cdcec Make sure objects/pack exists before creating a new pack
In a repository created with git older than f49fb35 (git-init-db: create
"pack" subdirectory under objects, 2005-06-27), objects/pack/ directory is
not created upon initialization.  It was Ok because subdirectories are
created as needed inside directories init-db creates, and back then,
packfiles were recent invention.

After the said commit, new codepaths started relying on the presense of
objects/pack/ directory in the repository.  This was exacerbated with
8b4eb6b (Do not perform cross-directory renames when creating packs,
2008-09-22) that moved the location temporary pack files are created from
objects/ directory to objects/pack/ directory, because moving temporary to
the final location was done carefully with lazy leading directory creation.

Many packfile related operations in such an old repository can fail
mysteriously because of this.

This commit introduces two helper functions to make things work better.

 - odb_mkstemp() is a specialized version of mkstemp() to refactor the
   code and teach it to create leading directories as needed;

 - odb_pack_keep() refactors the code to create a ".keep" file while
   create leading directories as needed.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-02-25 14:39:42 -08:00
Linus Torvalds
39c68542fc Wrap inflate and other zlib routines for better error reporting
R. Tyler Ballance reported a mysterious transient repository corruption;
after much digging, it turns out that we were not catching and reporting
memory allocation errors from some calls we make to zlib.

This one _just_ wraps things; it doesn't do the "retry on low memory
error" part, at least not yet. It is an independent issue from the
reporting.  Some of the errors are expected and passed back to the caller,
but we die when zlib reports it failed to allocate memory for now.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2009-01-11 02:13:06 -08:00
Junio C Hamano
559e840b74 Move read_in_full() and write_in_full() to wrapper.c
A few compat/* layer functions call these functions, but we would really
want to keep them thin, without depending too much on the libgit proper.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-07-20 17:17:09 -07:00
Linus Torvalds
112db553b0 Shrink the git binary a bit by avoiding unnecessary inline functions
So I was looking at the disgusting size of the git binary, and even with
the debugging removed, and using -Os instead of -O2, the size of the text
section was pretty high. In this day and age I guess almost a megabyte of
text isn't really all that surprising, but it still doesn't exactly make
me think "lean and mean".

With -Os, a surprising amount of text space is wasted on inline functions
that end up just being replicated multiple times, and where performance
really isn't a valid reason to inline them. In particular, the trivial
wrapper functions like "xmalloc()" are used _everywhere_, and making them
inline just duplicates the text (and the string we use to 'die()' on
failure) unnecessarily.

So this just moves them into a "wrapper.c" file, getting rid of a tiny bit
of unnecessary bloat. The following numbers are both with "CFLAGS=-Os":

Before:
	[torvalds@woody git]$ size git
	   text	   data	    bss	    dec	    hex	filename
	 700460	  15160	 292184	1007804	  f60bc	git

After:
	[torvalds@woody git]$ size git
	   text	   data	    bss	    dec	    hex	filename
	 670540	  15160	 292184	 977884	  eebdc	git

so it saves almost 30k of text-space (it actually saves more than that
with the default -O2, but I don't think that's necessarily a very relevant
number from a "try to shrink git" standpoint).

It might conceivably have a performance impact, but none of this should be
_that_ performance critical. The real cost is not generally in the wrapper
anyway, but in the code it wraps (ie the cost of "xread()" is all in the
read itself, not in the trivial wrapping of it).

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2008-06-22 18:46:00 -07:00