From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org, Karl Moskowski <kmoskowski@me.com>,
Mike Hommey <mh@glandium.org>,
David Turner <dturner@twopensource.com>
Subject: Re: [PATCH 01/20] safe_create_leading_directories_const(): preserve errno
Date: Tue, 16 Feb 2016 18:45:00 -0500 [thread overview]
Message-ID: <20160216234500.GA30631@sigill.intra.peff.net> (raw)
In-Reply-To: <1ff7b215e37593e935d8db81ea43c3c864161f71.1455626201.git.mhagger@alum.mit.edu>
On Tue, Feb 16, 2016 at 02:22:14PM +0100, Michael Haggerty wrote:
> Theoretically, free() is allowed to change errno. So preserve the errno
> from safe_create_leading_directories() across the call to free().
I wondered if this was actually a problem in practice. POSIX forbids
recreational setting of errno to "0" , and there are no errors defined
for free(). But apparently it is indeed a thing:
https://sourceware.org/bugzilla/show_bug.cgi?id=17924
and our desired behavior will even become the standard in a future
POSIX (not that we can rely on that for a long time...).
I suspect in practice that most allocators would use the sbrk() heap for
a small allocation like this, so we wouldn't run into any munmap()
errors during the free. But obviously that's being way more intimate
with the allocator than we should be, and your patch is a simple
protection that works either way.
-Peff
next prev parent reply other threads:[~2016-02-16 23:45 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-16 13:22 [PATCH 00/20] Delete directories left empty after ref deletion Michael Haggerty
2016-02-16 13:22 ` [PATCH 01/20] safe_create_leading_directories_const(): preserve errno Michael Haggerty
2016-02-16 23:45 ` Jeff King [this message]
2016-02-16 13:22 ` [PATCH 02/20] safe_create_leading_directories(): set errno on SCLD_EXISTS Michael Haggerty
2016-02-17 19:23 ` Junio C Hamano
2016-02-18 15:33 ` Michael Haggerty
2016-02-16 13:22 ` [PATCH 03/20] raceproof_create_file(): new function Michael Haggerty
2016-02-17 19:38 ` Junio C Hamano
2016-02-16 13:22 ` [PATCH 04/20] lock_ref_sha1_basic(): use raceproof_create_file() Michael Haggerty
2016-02-17 20:44 ` Junio C Hamano
2016-02-16 13:22 ` [PATCH 05/20] rename_tmp_log(): " Michael Haggerty
2016-02-17 20:53 ` Junio C Hamano
2016-02-19 16:07 ` Michael Haggerty
2016-02-19 17:15 ` Junio C Hamano
2016-02-16 13:22 ` [PATCH 06/20] rename_tmp_log(): improve error reporting Michael Haggerty
2016-02-18 22:14 ` Junio C Hamano
2016-02-16 13:22 ` [PATCH 07/20] log_ref_setup(): separate code for create vs non-create Michael Haggerty
2016-02-16 13:22 ` [PATCH 08/20] log_ref_setup(): improve robustness against races Michael Haggerty
2016-02-18 22:17 ` Junio C Hamano
2016-02-16 13:22 ` [PATCH 09/20] log_ref_setup(): pass the open file descriptor back to the caller Michael Haggerty
2016-02-18 22:21 ` Junio C Hamano
2016-02-16 13:22 ` [PATCH 10/20] log_ref_write_1(): don't depend on logfile Michael Haggerty
2016-02-16 13:22 ` [PATCH 11/20] log_ref_setup(): manage the name of the reflog file internally Michael Haggerty
2016-02-16 13:22 ` [PATCH 12/20] log_ref_write_1(): inline function Michael Haggerty
2016-02-18 22:23 ` Junio C Hamano
2016-02-16 13:22 ` [PATCH 13/20] try_remove_empty_parents(): rename parameter "name" -> "refname" Michael Haggerty
2016-02-16 13:22 ` [PATCH 14/20] try_remove_empty_parents(): don't trash argument contents Michael Haggerty
2016-02-16 13:22 ` [PATCH 15/20] try_remove_empty_parents(): don't accommodate consecutive slashes Michael Haggerty
2016-02-16 13:22 ` [PATCH 16/20] t5505: use "for-each-ref" to test for the non-existence of references Michael Haggerty
2016-02-16 13:22 ` [PATCH 17/20] delete_ref_loose(): derive loose reference path from lock Michael Haggerty
2016-02-16 13:22 ` [PATCH 18/20] delete_ref_loose(): inline function Michael Haggerty
2016-02-16 13:22 ` [PATCH 19/20] try_remove_empty_parents(): teach to remove parents of reflogs, too Michael Haggerty
2016-02-16 13:22 ` [PATCH 20/20] ref_transaction_commit(): clean up empty directories Michael Haggerty
2016-02-17 0:08 ` [PATCH 00/20] Delete directories left empty after ref deletion Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160216234500.GA30631@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=dturner@twopensource.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=kmoskowski@me.com \
--cc=mh@glandium.org \
--cc=mhagger@alum.mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).