[PATCH 2/7] make PED_ASSERT handling sane: abort on false condition
Jim Meyering
meyering at redhat.com
Wed Sep 30 09:10:15 UTC 2009
Upon a failed PED_ASSERT, Do not ask the interactive user if they want
to continue. This change will affect any code that expects to be able
to continue after a failed PED_ASSERT condition. However, such code is
so fundamentally broken that this change is required. If you require
to be able to continue after a false condition, then the code must
not use a macro named like PED_ASSERT. This change was motivated by
my desire to use the clang static analysis tool. Without this change,
there were over 300 mostly-false-positive reports. With it, just 31,
almost all legitimate.
* libparted/debug.c (ped_assert): Remove first parameter.
It is now tested via the macro.
Now, this function always aborts, which helps clang understand.
Do not ask the interactive user if s/he wants to continue.
* include/parted/debug.h (PED_ASSERT): Test condition here,
not in ped_assert.
* libparted/disk.c (ped_disk_remove_partition): Remove the label
that was used only upon failing PED_ASSERT. Now that PED_ASSERT
always aborts for a false condition, that would have been dead code.
(ped_disk_new_fresh): Likewise.
---
include/parted/debug.h | 23 +++++++++++------------
libparted/debug.c | 17 +++++------------
libparted/disk.c | 10 ++--------
3 files changed, 18 insertions(+), 32 deletions(-)
diff --git a/include/parted/debug.h b/include/parted/debug.h
index 4fcabb7..0195ac2 100644
--- a/include/parted/debug.h
+++ b/include/parted/debug.h
@@ -30,8 +30,9 @@ extern void ped_debug_set_handler (PedDebugHandler* handler);
extern void ped_debug ( const int level, const char* file, int line,
const char* function, const char* msg, ... );
-extern int ped_assert ( int cond, const char* cond_text,
- const char* file, int line, const char* function );
+extern void __attribute__((__noreturn__))
+ped_assert ( const char* cond_text,
+ const char* file, int line, const char* function );
#if defined __GNUC__ && !defined __JSFTRACE__
@@ -41,14 +42,13 @@ extern int ped_assert ( int cond, const char* cond_text,
#define PED_ASSERT(cond, action) \
do { \
- if (!ped_assert ( cond, \
+ if (!(cond)) { \
+ ped_assert ( \
#cond, \
__FILE__, \
__LINE__, \
- __PRETTY_FUNCTION__ )) \
- { \
- action; \
- } \
+ __PRETTY_FUNCTION__ ); \
+ } \
} while (0)
#else /* !__GNUC__ */
@@ -65,14 +65,13 @@ static void PED_DEBUG (int level, ...)
#define PED_ASSERT(cond, action) \
do { \
- if (!ped_assert ( cond, \
+ if (!(cond)) { \
+ ped_assert ( \
#cond, \
"unknown", \
0, \
- "unknown" )) \
- { \
- action; \
- } \
+ "unknown"); \
+ } \
} while (0)
#endif /* __GNUC__ */
diff --git a/libparted/debug.c b/libparted/debug.c
index 564520a..86798c3 100644
--- a/libparted/debug.c
+++ b/libparted/debug.c
@@ -82,14 +82,9 @@ void ped_debug_set_handler ( PedDebugHandler* handler )
* Check an assertion.
* Do not call this directly -- use PED_ASSERT() instead.
*/
-int ped_assert (int cond, const char* cond_text,
- const char* file, int line, const char* function)
+void ped_assert (const char* cond_text,
+ const char* file, int line, const char* function)
{
- PedExceptionOption opt;
-
- if (cond)
- return 1;
-
#if HAVE_BACKTRACE
/* Print backtrace stack */
void *stack[20];
@@ -108,14 +103,12 @@ int ped_assert (int cond, const char* cond_text,
#endif
/* Throw the exception */
- opt = ped_exception_throw (
+ ped_exception_throw (
PED_EXCEPTION_BUG,
- PED_EXCEPTION_IGNORE_CANCEL,
+ PED_EXCEPTION_FATAL,
_("Assertion (%s) at %s:%d in function %s() failed."),
cond_text, file, line, function);
-
- return (opt == PED_EXCEPTION_IGNORE);
+ abort ();
}
#endif /* DEBUG */
-
diff --git a/libparted/disk.c b/libparted/disk.c
index f3ad134..f3074a3 100644
--- a/libparted/disk.c
+++ b/libparted/disk.c
@@ -359,13 +359,11 @@ ped_disk_new_fresh (PedDevice* dev, const PedDiskType* type)
if (!disk)
goto error;
_disk_pop_update_mode (disk);
- PED_ASSERT (disk->update_mode == 0, goto error_destroy_disk);
+ PED_ASSERT (disk->update_mode == 0, ignored);
disk->needs_clobber = 1;
return disk;
-error_destroy_disk:
- ped_disk_destroy (disk);
error:
return NULL;
}
@@ -1849,15 +1847,11 @@ ped_disk_remove_partition (PedDisk* disk, PedPartition* part)
PED_ASSERT (part != NULL, return 0);
_disk_push_update_mode (disk);
- PED_ASSERT (part->part_list == NULL, goto error);
+ PED_ASSERT (part->part_list == NULL, ignored);
_disk_raw_remove (disk, part);
_disk_pop_update_mode (disk);
ped_disk_enumerate_partitions (disk);
return 1;
-
-error:
- _disk_pop_update_mode (disk);
- return 0;
}
static int
--
1.6.5.rc2.177.ga9dd6
More information about the parted-devel
mailing list