- Patch #8973 by JonBob: Drupal contains many undefined variables and array indices, which makes PHP throw a lot of warnings when the reporting level is set to E_ALL. Things run fine with these warnings, but as a matter of code style if nothing else we should probably strive to avoid them. The attached fixes most of the more egregious offenders (about 95% of the warnings when I load /node on my test site).

11 files changed, 79 insertions, 53 deletions

diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc
index 2f3e8dbfe..968f0d29f 100644
--- a/includes/bootstrap.inc
+++ b/includes/bootstrap.inc
@@ -204,7 +204,9 @@ function arg($index) {
     $arguments = explode('/', $_GET['q']);
   }
 
-  return $arguments[$index];
+  if (array_key_exists($index, $arguments)) {
+    return $arguments[$index];
+  }
 }
 
 function check_query($text) {
diff --git a/includes/common.inc b/includes/common.inc
index efe0fd4e3..48d2a3e01 100644
--- a/includes/common.inc
+++ b/includes/common.inc
@@ -234,7 +234,7 @@ function drupal_not_found() {
   watchdog('httpd', t('404 error: "%page" not found', array('%page' => check_query($_GET["q"]))));
 
   $path = drupal_get_normal_path(variable_get('site_404', ''));
-  $status = MENU_FALLTHROUGH;
+  $status = MENU_NOT_FOUND;
   if ($path) {
     menu_set_active_item($path);
     $status = menu_execute_active_handler();
@@ -252,7 +252,7 @@ function drupal_access_denied() {
   header('HTTP/1.0 403 Forbidden');
 
   $path = drupal_get_normal_path(variable_get('site_403', ''));
-  $status = MENU_FALLTHROUGH;
+  $status = MENU_NOT_FOUND;
   if ($path) {
     menu_set_active_item($path);
     $status = menu_execute_active_handler();
@@ -866,7 +866,8 @@ function format_date($timestamp, $type = 'medium', $format = '', $timezone = NUL
 
   $max = strlen($format);
   $date = '';
-  for ($i = 0; $i <= $max; $c = $format{$i++}) {
+  for ($i = 0; $i < $max; $i++) {
+    $c = $format{$i};
     if (strpos('AaDFlM', $c)) {
       $date .= t(gmdate($c, $timestamp));
     }
@@ -963,14 +964,16 @@ function form_set_error($name, $message) {
  * Return true when errors have been set.
  */
 function form_has_errors() {
-  return isset($GLOBALS['form']);
+  return array_key_exists('form', $GLOBALS);
 }
 
 /**
  * Return the error message filed against the form with the specified name.
  */
 function _form_get_error($name) {
-  return $GLOBALS['form'][$name];
+  if (array_key_exists('form', $GLOBALS)) {
+    return $GLOBALS['form'][$name];
+  }
 }
 
 function _form_get_class($name, $required, $error) {
diff --git a/includes/menu.inc b/includes/menu.inc
index f6d12bf50..330a56c69 100644
--- a/includes/menu.inc
+++ b/includes/menu.inc
@@ -200,12 +200,15 @@ function menu_execute_active_handler() {
 
   // Determine the menu item containing the callback.
   $path = $_GET['q'];
-  while ($path && (!$menu['path index'][$path] || !$menu['items'][$menu['path index'][$path]]['callback'])) {
+  while ($path && (!array_key_exists($path, $menu['path index']) || empty($menu['items'][$menu['path index'][$path]]['callback']))) {
     $path = substr($path, 0, strrpos($path, '/'));
   }
+  if (!array_key_exists($path, $menu['path index'])) {
+    return MENU_NOT_FOUND;
+  }
   $mid = $menu['path index'][$path];
 
-  if (!is_string($menu['items'][$mid]['callback'])) {
+  if (empty($menu['items'][$mid]['callback'])) {
     return MENU_NOT_FOUND;
   }
 
@@ -214,7 +217,7 @@ function menu_execute_active_handler() {
   }
 
   // We found one, and are allowed to execute it.
-  $arguments = $menu['items'][$mid]['callback arguments'] ? $menu['items'][$mid]['callback arguments'] : array();
+  $arguments = $menu['items'][$mid]['callback arguments'];
   $arg = substr($_GET['q'], strlen($menu['items'][$mid]['path']) + 1);
   if (strlen($arg)) {
     $arguments = array_merge($arguments, explode('/', $arg));
@@ -245,10 +248,10 @@ function menu_set_active_item($path = NULL) {
       $_GET['q'] = $path;
     }
 
-    while ($path && !$menu['path index'][$path]) {
+    while ($path && !array_key_exists($path, $menu['path index'])) {
       $path = substr($path, 0, strrpos($path, '/'));
     }
-    $stored_mid = $menu['path index'][$path];
+    $stored_mid = array_key_exists($path, $menu['path index']) ? $menu['path index'][$path] : 0;
   }
 
   return $stored_mid;
@@ -483,9 +486,12 @@ function _menu_get_trail($path) {
   $trail = array();
 
   // Find the ID of the given path.
-  while ($path && !$menu['path index'][$path]) {
+  while ($path && !array_key_exists($path, $menu['path index'])) {
     $path = substr($path, 0, strrpos($path, '/'));
   }
+  if (!array_key_exists($path, $menu['path index'])) {
+    return array();
+  }
   $mid = $menu['path index'][$path];
 
   // Follow the parents up the chain to get the trail.
@@ -522,8 +528,8 @@ function _menu_build() {
   $_menu['path index'] = array();
   // Set up items array, including default "Navigation" menu.
   $_menu['items'] = array(
-    0 => array('type' => MENU_IS_ROOT),
-    1 => array('pid' => 0, 'title' => t('Navigation'), 'weight' => -50, 'access' => TRUE, 'type' => MENU_IS_ROOT | MENU_VISIBLE_IN_TREE)
+    0 => array('path' => '', 'title' => '', 'type' => MENU_IS_ROOT),
+    1 => array('pid' => 0, 'path' => '', 'title' => t('Navigation'), 'weight' => -50, 'access' => TRUE, 'type' => MENU_IS_ROOT | MENU_VISIBLE_IN_TREE)
     );
 
   // Build a sequential list of all menu items.
@@ -533,11 +539,20 @@ function _menu_build() {
   $temp_mid = -1;
 
   foreach ($menu_item_list as $item) {
-    if (!isset($item['type'])) {
+    if (!array_key_exists('path', $item)) {
+      $item['path'] = '';
+    }
+    if (!array_key_exists('type', $item)) {
       $item['type'] = MENU_NORMAL_ITEM;
     }
+    if (!array_key_exists('weight', $item)) {
+      $item['weight'] = 0;
+    }
+    if (!array_key_exists('callback arguments', $item)) {
+      $item['callback arguments'] = array();
+    }
     $mid = $temp_mid;
-    if (isset($_menu['path index'][$item['path']])) {
+    if (array_key_exists($item['path'], $_menu['path index'])) {
       // Newer menu items overwrite older ones.
       unset($_menu['items'][$_menu['path index'][$item['path']]]);
     }
@@ -552,7 +567,8 @@ function _menu_build() {
     $result = db_query('SELECT * FROM {menu}');
     while ($item = db_fetch_object($result)) {
       // Don't display non-custom menu items if no module declared them.
-      if ($old_mid = $_menu['path index'][$item->path]) {
+      if (array_key_exists($item->path, $_menu['path index'])) {
+        $old_mid = $_menu['path index'][$item->path];
         $_menu['items'][$item->mid] = $_menu['items'][$old_mid];
         unset($_menu['items'][$old_mid]);
         $_menu['path index'][$item->path] = $item->mid;
@@ -566,7 +582,7 @@ function _menu_build() {
       }
       // Next, add any custom items added by the administrator.
       else if ($item->type & MENU_CREATED_BY_ADMIN) {
-        $_menu['items'][$item->mid] = array('pid' => $item->pid, 'path' => $item->path, 'title' => $item->title, 'access' => TRUE, 'weight' => $item->weight, 'type' => $item->type);
+        $_menu['items'][$item->mid] = array('pid' => $item->pid, 'path' => $item->path, 'title' => $item->title, 'access' => TRUE, 'weight' => $item->weight, 'type' => $item->type, 'callback' => '', 'callback arguments' => array());
         $_menu['path index'][$item->path] = $item->mid;
       }
     }
@@ -580,7 +596,7 @@ function _menu_build() {
       do {
         $parent = substr($parent, 0, strrpos($parent, '/'));
       }
-      while ($parent && !$_menu['path index'][$parent]);
+      while ($parent && !array_key_exists($parent, $_menu['path index']));
 
       $pid = $parent ? $_menu['path index'][$parent] : 1;
       $_menu['items'][$mid]['pid'] = $pid;
@@ -615,15 +631,18 @@ function _menu_build() {
 function _menu_item_is_accessible($mid) {
   $menu = menu_get_menu();
 
-  if (isset($menu['items'][$mid]['access'])) {
+  if (array_key_exists('access', $menu['items'][$mid])) {
     return $menu['items'][$mid]['access'];
   }
 
   // Follow the path up to find the actual callback.
   $path = $menu['items'][$mid]['path'];
-  while ($path && (!$menu['path index'][$path] || !$menu['items'][$menu['path index'][$path]]['callback'])) {
+  while ($path && (!array_key_exists($path, $menu['path index']) || !array_key_exists('callback', $menu['items'][$menu['path index'][$path]]))) {
     $path = substr($path, 0, strrpos($path, '/'));
   }
+  if (empty($path)) {
+    return FALSE;
+  }
   $callback_mid = $menu['path index'][$path];
   return $menu['items'][$callback_mid]['access'];
 }
@@ -641,7 +660,7 @@ function _menu_build_visible_tree($pid = 0) {
     $parent = $_menu['items'][$pid];
 
     $children = array();
-    if ($parent['children']) {
+    if (array_key_exists('children', $parent)) {
       usort($parent['children'], '_menu_sort');
       foreach ($parent['children'] as $mid) {
         $children = array_merge($children, _menu_build_visible_tree($mid));
@@ -679,31 +698,33 @@ function _menu_build_local_tasks() {
   $tasks[0] = array('children' => array());
 
   $mid = menu_get_active_nontask_item();
-  $tasks[$mid] = array('title' => $_menu['items'][$mid]['title'], 'path' => $_menu['items'][$mid]['path'], 'children' => array());
-  $tasks[0]['children'][] = $mid;
-
-  // Find top-level tasks
-  if ($children = $_menu['items'][$mid]['children']) {
-    foreach ($children as $cid) {
-      if (($_menu['items'][$cid]['type'] & MENU_IS_LOCAL_TASK) && _menu_item_is_accessible($cid)) {
-        $tasks[$cid] = array('title' => $_menu['items'][$cid]['title'], 'path' => $_menu['items'][$cid]['path'], 'children' => array());
-        $tasks[0]['children'][] = $cid;
-      }
-    }
-  }
-  usort($tasks[0]['children'], '_menu_sort');
+  if ($mid) {
+    $tasks[$mid] = array('title' => $_menu['items'][$mid]['title'], 'path' => $_menu['items'][$mid]['path'], 'children' => array());
+    $tasks[0]['children'][] = $mid;
 
-  // Find subtasks
-  foreach ($tasks[0]['children'] as $mid) {
+    // Find top-level tasks
     if ($children = $_menu['items'][$mid]['children']) {
       foreach ($children as $cid) {
-        if (($_menu['items'][$cid]['type'] & MENU_IS_LOCAL_SUBTASK) && _menu_item_is_accessible($cid)) {
+        if (($_menu['items'][$cid]['type'] & MENU_IS_LOCAL_TASK) && _menu_item_is_accessible($cid)) {
           $tasks[$cid] = array('title' => $_menu['items'][$cid]['title'], 'path' => $_menu['items'][$cid]['path'], 'children' => array());
-          $tasks[$mid]['children'][] = $cid;
+          $tasks[0]['children'][] = $cid;
+        }
+      }
+    }
+    usort($tasks[0]['children'], '_menu_sort');
+
+    // Find subtasks
+    foreach ($tasks[0]['children'] as $mid) {
+      if ($children = $_menu['items'][$mid]['children']) {
+        foreach ($children as $cid) {
+          if (($_menu['items'][$cid]['type'] & MENU_IS_LOCAL_SUBTASK) && _menu_item_is_accessible($cid)) {
+            $tasks[$cid] = array('title' => $_menu['items'][$cid]['title'], 'path' => $_menu['items'][$cid]['path'], 'children' => array());
+            $tasks[$mid]['children'][] = $cid;
+          }
         }
       }
+      usort($tasks[$mid]['children'], '_menu_sort');
     }
-    usort($tasks[$mid]['children'], '_menu_sort');
   }
 
   if (count($tasks) > 2) {
diff --git a/includes/pager.inc b/includes/pager.inc
index 40abb7d2b..837f8e2fa 100644
--- a/includes/pager.inc
+++ b/includes/pager.inc
@@ -70,6 +70,8 @@ function pager_query($query, $limit = 10, $element = 0, $count_query = "") {
  */
 function theme_pager($tags = "", $limit = 10, $element = 0, $attributes = array()) {
   global $pager_total;
+  $output = '';
+
   if ($pager_total[$element] > $limit) {
     $output .= "<div id=\"pager\" class=\"container-inline\">";
     $output .= "<div>". pager_first(($tags[0] ? $tags[0] : t("first page")), $limit, $element, $attributes) ."</div>";
@@ -282,8 +284,8 @@ function pager_list($limit, $element = 0, $quantity = 5, $text = "", $attributes
 /* @} End of member group pager pieces */
 
 function pager_link($from_new, $element, $attributes = array()) {
-  $q = $_GET["q"];
-  $from = $_GET["from"];
+  $q = $_GET['q'];
+  $from = array_key_exists('from', $_GET) ? $_GET['from'] : '';
 
   foreach($attributes as $key => $value) {
     $query[] = "$key=$value";
diff --git a/includes/theme.inc b/includes/theme.inc
index 339ec3512..494ee78b2 100644
--- a/includes/theme.inc
+++ b/includes/theme.inc
@@ -46,7 +46,7 @@ function init_theme() {
   ** list of enabled themes.
   */
 
-  $theme = $themes[$user->theme] ? $user->theme : variable_get("theme_default", 0);
+  $theme = $user->theme && $themes[$user->theme] ? $user->theme : variable_get("theme_default", 0);
 
   include_once($themes[$theme]->filename);
 
@@ -519,6 +519,8 @@ function theme_blocks($region) {
 /* @} */
 
 function _theme_table_cell($cell, $header = 0) {
+  $attributes = '';
+
   if (is_array($cell)) {
     $data = $cell['data'];
     foreach ($cell as $key => $value) {
diff --git a/modules/help.module b/modules/help.module
index 6cfdce90c..f386c177a 100644
--- a/modules/help.module
+++ b/modules/help.module
@@ -47,10 +47,8 @@ function help_glossary() {
 function help_help($section) {
   switch ($section) {
     case 'admin/modules#description':
-      $output = t('Manages displaying online help.');
-      break;
+      return t('Manages displaying online help.');
   }
-  return $output;
 }
 
 /**
diff --git a/modules/help/help.module b/modules/help/help.module
index 6cfdce90c..f386c177a 100644
--- a/modules/help/help.module
+++ b/modules/help/help.module
@@ -47,10 +47,8 @@ function help_glossary() {
 function help_help($section) {
   switch ($section) {
     case 'admin/modules#description':
-      $output = t('Manages displaying online help.');
-      break;
+      return t('Manages displaying online help.');
   }
-  return $output;
 }
 
 /**
diff --git a/modules/node.module b/modules/node.module
index 3ea1aa664..7ec3e4c1e 100644
--- a/modules/node.module
+++ b/modules/node.module
@@ -620,7 +620,7 @@ function node_link($type, $node = 0, $main = 0) {
   $links = array();
 
   if ($type == 'node') {
-    if ($node->links) {
+    if (array_key_exists('links', $node)) {
       $links = $node->links;
     }
 
diff --git a/modules/node/node.module b/modules/node/node.module
index 3ea1aa664..7ec3e4c1e 100644
--- a/modules/node/node.module
+++ b/modules/node/node.module
@@ -620,7 +620,7 @@ function node_link($type, $node = 0, $main = 0) {
   $links = array();
 
   if ($type == 'node') {
-    if ($node->links) {
+    if (array_key_exists('links', $node)) {
       $links = $node->links;
     }
 
diff --git a/modules/taxonomy.module b/modules/taxonomy.module
index f8c4226e5..8bd8ba1f1 100644
--- a/modules/taxonomy.module
+++ b/modules/taxonomy.module
@@ -38,7 +38,7 @@ function taxonomy_perm() {
 function taxonomy_link($type, $node = NULL) {
   if ($type == 'taxonomy terms' && $node != NULL) {
     $links = array();
-    if ($node->taxonomy) {
+    if (array_key_exists('taxonomy', $node)) {
       foreach ($node->taxonomy as $tid) {
         $term = taxonomy_get_term($tid);
         $links[] = l($term->name, "taxonomy/page/or/$term->tid", $term->description ? array ('title' => $term->description) : array());
diff --git a/modules/taxonomy/taxonomy.module b/modules/taxonomy/taxonomy.module
index f8c4226e5..8bd8ba1f1 100644
--- a/modules/taxonomy/taxonomy.module
+++ b/modules/taxonomy/taxonomy.module
@@ -38,7 +38,7 @@ function taxonomy_perm() {
 function taxonomy_link($type, $node = NULL) {
   if ($type == 'taxonomy terms' && $node != NULL) {
     $links = array();
-    if ($node->taxonomy) {
+    if (array_key_exists('taxonomy', $node)) {
       foreach ($node->taxonomy as $tid) {
         $term = taxonomy_get_term($tid);
         $links[] = l($term->name, "taxonomy/page/or/$term->tid", $term->description ? array ('title' => $term->description) : array());