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>
* 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
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
