[systemd-commits] 2 commits - src/bootchart src/shared

Kay Sievers kay at kemper.freedesktop.org
Mon Dec 30 10:11:51 PST 2013


 src/bootchart/store.c     |    2 +-
 src/shared/sleep-config.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

New commits:
commit 34a3baa4dbd8a4032ae74cb5947b9494bf3ec106
Author: Stefan Beller <stefanbeller at googlemail.com>
Date:   Mon Dec 30 17:43:52 2013 +0100

    sleep-config: Dereference pointer before check for NULL
    
    This fixes a bug pointed out by http://css.csail.mit.edu/stack/
    (Optimization-unstable code)
    It is a similar fix as f146f5e159 (2013-12-30, core:
    Forgot to dereference pointer when checking for NULL)
    
    To explain this bug consider the following similar, but simpler code:
    	if (!p)
    		free(*p)
    
    Assume the if condition evaluates to true, then we will access *p,
    which means the compiler can assume p is a valid pointer, so it could
    dereference p and use the value *p.
    Assuming p as a valid pointer, !p will be false.
    But initally we assumed the condition evaluates to true.
    
    By this reasoning the optimizing compiler can deduce, we have dead code.
    ("The if will never be taken, as *p must be valid, because otherwise
    accessing *p inside the if would segfault")
    
    This led to an error message of the static code checker, so I checked the
    code in question.
    
    As we access *modes and *states before the check in the changed line of
    this patch, I assume the line to be wrong and we actually wanted to check
    for *modes and *states being both non null.

diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c
index d76e3ad..b2a0787 100644
--- a/src/shared/sleep-config.c
+++ b/src/shared/sleep-config.c
@@ -94,7 +94,7 @@ int parse_sleep_config(const char *verb, char ***modes, char ***states) {
         } else
                 assert_not_reached("what verb");
 
-        if (!modes || !states) {
+        if (!*modes || !*states) {
                 strv_free(*modes);
                 strv_free(*states);
                 return log_oom();

commit 226b735a745c1e9bbab24b47460d7304def9d38f
Author: Stefan Beller <stefanbeller at googlemail.com>
Date:   Mon Dec 30 00:09:56 2013 +0100

    bootchart: Remove unneeded check for NULL
    
    Directly before the changed line there is:
    
    	while ((parent->next_ps && parent->pid != ps->ppid))
    		parent = parent->next_ps;
    
    which looks one element ahead of the list, hence we can rely on parent
    being non null here.
    If 'parent' were NULL at that while loop already, it would crash as we're
    dereferencing 'parent' when checking for next_ps already.
    
    Signed-off-by: Stefan Beller <stefanbeller at googlemail.com>

diff --git a/src/bootchart/store.c b/src/bootchart/store.c
index 7f86cfe..b6ba113 100644
--- a/src/bootchart/store.c
+++ b/src/bootchart/store.c
@@ -345,7 +345,7 @@ schedstat_next:
                         while ((parent->next_ps && parent->pid != ps->ppid))
                                 parent = parent->next_ps;
 
-                        if ((!parent) || (parent->pid != ps->ppid)) {
+                        if (parent->pid != ps->ppid) {
                                 /* orphan */
                                 ps->ppid = 1;
                                 parent = ps_first->next_ps;



More information about the systemd-commits mailing list