linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Theodore Ts'o <tytso@mit.edu>
To: Andreas Dilger <adilger@whamcloud.com>
Cc: Ext4 Developers List <linux-ext4@vger.kernel.org>,
	"tony@bakeyournoodle.com" <tony@bakeyournoodle.com>
Subject: Re: [PATCH 2/7] libext2fs: use abort() instead of perror()/exit()
Date: Tue, 31 Jul 2012 16:04:58 -0400	[thread overview]
Message-ID: <20120731200458.GE32228@thunk.org> (raw)
In-Reply-To: <C4F449DF-59AB-4E63-9545-9D2F8F912563@whamcloud.com>

On Tue, Jul 31, 2012 at 11:34:38AM -0700, Andreas Dilger wrote:
> On 2012-07-30, at 14:47, Theodore Ts'o <tytso@mit.edu> wrote:
> 
> > This simplifies the number of C library symbols needed by boot loader
> > systems such as yaboot.
> 
> This doesn't improve the debugability of the code at all. Instead of
> getting an error message (as cryptic as it was), now there is no
> error and the process will just die.

Well, at least for e2fsck, which is the program I was most concerned
about, the debuggability will actually improve, since
e2fsck/sigcatcher.c will give you a very nice stack backtrace (at
least, if your libc has the backtrace function).

> I'm guessing from the original coding that there is no error
> handling for this case?

Yes, the problem is that the ext2fs_{mark,unmark}_{block,inode}_bitmap()
functions return void, and changing this would require massive changes
all up and down the stack.

Even if they had originally return an errcode_t, given that with the
simple bit array implementation, they could Never Fail(tm), it's
likely that most if not all of the code sites would not have checked
them, and even if they did, all they could really do at that point is
die.  And if they didn't, then it would be even harder to debug why
the bitmap function was became a no-op due to a memory allocation
failure.

Sigh; I've become convinced that the Go language's philosphy not
letting memory allocation fail (and just simply dying if you can't
allocate the memory you need) is the Right Thing 99.99% of the time.


						- Ted

  reply	other threads:[~2012-07-31 20:05 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-15  4:24 Minimal configuration for e2fsprogs Tony Breeds
2012-06-16  0:08 ` Ted Ts'o
2012-06-18  5:58   ` Tony Breeds
2012-06-18 17:12     ` Ted Ts'o
2012-06-19  5:48       ` Tony Breeds
2012-06-19  6:01         ` Andreas Dilger
2012-06-19 13:56           ` Ted Ts'o
2012-06-20  5:26             ` Tony Breeds
2012-06-20  5:33               ` Tony Breeds
2012-06-20 14:14               ` Andreas Dilger
2012-06-20  4:45           ` Tony Breeds
2012-06-26  2:10       ` Tony Breeds
2012-06-26  2:33         ` Theodore Ts'o
2012-06-26  2:47           ` Tony Breeds
2012-06-27 11:21 ` Tony Breeds
2012-06-27 12:54   ` Theodore Ts'o
2012-06-28  2:43     ` Tony Breeds
2012-07-30 21:45       ` Theodore Ts'o
2012-07-31  5:21         ` Tony Breeds
2012-07-31 19:57           ` Theodore Ts'o
2012-08-01  5:42             ` Tony Breeds
2012-07-30 21:47       ` [PATCH 1/7] e2fsck: add SIGABRT to list of signals processed by sigcatcher Theodore Ts'o
2012-07-30 21:47         ` [PATCH 2/7] libext2fs: use abort() instead of perror()/exit() Theodore Ts'o
2012-07-31 18:34           ` Andreas Dilger
2012-07-31 20:04             ` Theodore Ts'o [this message]
2012-07-30 21:47         ` [PATCH 3/7] libext2fs: use strcpy()/strcat() instead of sprintf() in bmap functions Theodore Ts'o
2012-07-30 21:47         ` [PATCH 4/7] libext2fs: move ext2fs_get_num_dirs to its own file Theodore Ts'o
2012-07-30 21:47         ` [PATCH 5/7] libext2fs: call numeric_progress functions through a operations struct Theodore Ts'o
2012-07-30 21:47         ` [PATCH 6/7] libext2fs: remove debugging printf from ext2fs_group_desc_csum Theodore Ts'o
2012-07-30 21:47         ` [PATCH 7/7] libext2fs: enforce the block group descriptor size in ext2fs_open() Theodore Ts'o
2012-07-31 18:38           ` Andreas Dilger
2012-07-31 20:09             ` Theodore Ts'o
2012-08-01 20:45               ` Andreas Dilger
2012-08-03  0:00                 ` Theodore Ts'o

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=20120731200458.GE32228@thunk.org \
    --to=tytso@mit.edu \
    --cc=adilger@whamcloud.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=tony@bakeyournoodle.com \
    /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).