[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