From 9986cb36b5cedc45b8ccfc9db8f4afa1d3a36675 Mon Sep 17 00:00:00 2001 From: Dries Buytaert <dries@buytaert.net> Date: Fri, 2 Jul 2004 18:46:42 +0000 Subject: [PATCH] - 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). --- includes/bootstrap.inc | 4 +- includes/common.inc | 13 +++-- includes/menu.inc | 89 ++++++++++++++++++++------------ includes/pager.inc | 6 ++- includes/theme.inc | 4 +- modules/help.module | 4 +- modules/help/help.module | 4 +- modules/node.module | 2 +- modules/node/node.module | 2 +- modules/taxonomy.module | 2 +- modules/taxonomy/taxonomy.module | 2 +- 11 files changed, 79 insertions(+), 53 deletions(-) diff --git a/includes/bootstrap.inc b/includes/bootstrap.inc index 2f3e8dbfeaa5..968f0d29f414 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 efe0fd4e3953..48d2a3e01a0d 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 f6d12bf50cda..330a56c698b2 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 40abb7d2b8a9..837f8e2fa4eb 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 339ec35122fb..494ee78b27ef 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 6cfdce90c098..f386c177ab3c 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 6cfdce90c098..f386c177ab3c 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 3ea1aa664b55..7ec3e4c1e8bf 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 3ea1aa664b55..7ec3e4c1e8bf 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 f8c4226e51f8..8bd8ba1f1248 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 f8c4226e51f8..8bd8ba1f1248 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()); -- GitLab