This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Empty if body in linux-record.c


Hi all,

this is one the issues reported by "clang --analyze" and it appears to
me that the code was broken in the same way since the initial check-in
of these routines, looking at the tests in gdb.record it does not appear
that these are covered at all.

I'm pretty sure that the code as is is wrong and that my patch is sensitive
but I'm not sure how to properly proof this. Currently the only way would
be to run a record an app that is using the syscalls and check if recording
fails/reports an issue and stops reporting it after wards?

I'm not really known to the patch contribution of gdb so I'm not sure if I'm
following the proper process here...anyway here is a patch with a
ChangeLog entry.


From 7e53969f18043d7cd997531934e850ccb6b95190 Mon Sep 17 00:00:00 2001
From: Holger Hans Peter Freyther <zecke@selfish.org>
Date: Sun, 14 Mar 2010 05:31:40 +0100
Subject: [PATCH] 2010-03-14  Holger Hans Peter Freyther  <zecke@selfish.org>

	* linux-record.c (record_linux_msghdr): Fix a compiler warning
	about an empty if body that has existed since the addition of the
	code. This change will change the return value of the function from
	always returning -1 to returning -1 only if calling record_arch_list_add_mem
	is failing. The inner for loop will also be iterated more than once
	now.
---
 gdb/ChangeLog      |    9 +++++++++
 gdb/linux-record.c |    4 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d9e6fd8..9f6528d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2010-03-14  Holger Hans Peter Freyther  <zecke@selfish.org>
+
+	* linux-record.c (record_linux_msghdr): Fix a compiler warning
+	about an empty if body that has existed since the addition of the
+	code. This change will change the return value of the function from
+	always returning -1 to returning -1 only if calling record_arch_list_add_mem
+	is failing. The inner for loop will also be iterated more than once
+	now.
+
 2010-03-12  Tom Tromey  <tromey@redhat.com>
 
 	PR c++/9708:
diff --git a/gdb/linux-record.c b/gdb/linux-record.c
index 1c6aa9f..74ef9bd 100644
--- a/gdb/linux-record.c
+++ b/gdb/linux-record.c
@@ -192,7 +192,7 @@ record_linux_msghdr (struct regcache *regcache,
           tmpint = (int) extract_unsigned_integer (iov + tdep->size_pointer,
                                                    tdep->size_size_t,
                                                    byte_order);
-          if (record_arch_list_add_mem (tmpaddr, tmpint));
+          if (record_arch_list_add_mem (tmpaddr, tmpint))
             return -1;
           addr += tdep->size_iovec;
         }
@@ -203,7 +203,7 @@ record_linux_msghdr (struct regcache *regcache,
   addr = extract_unsigned_integer (a, tdep->size_pointer, byte_order);
   a += tdep->size_pointer;
   tmpint = (int) extract_unsigned_integer (a, tdep->size_size_t, byte_order);
-  if (record_arch_list_add_mem ((CORE_ADDR) addr, tmpint));
+  if (record_arch_list_add_mem ((CORE_ADDR) addr, tmpint))
     return -1;
 
   return 0;
-- 
1.7.0




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]