project_issue.module produces various PHP Notices with E_ALL on. These are all due to variables, array keys or object fields which may not exist when used.

This patch fixes those that I spotted, by changing to equivalent behaviour without Notices.

Examples:
1.

  if (is_array($node->components)) {

changed to

  if (isset($node->components) && is_array($node->components)) {

2.

    if ($_GET['edit'][$field]) {

changed to

    if (!empty($_GET['edit'][$field])) {

3.

  '#default_value' => $node->mail_copy_filter[$key],

changed to

  '#default_value' => isset($node->mail_copy_filter[$key]) ? $node->mail_copy_filter[$key] : NULL,

4. Immediate bail from project_issue_project_load() if not found in table {project_issue_projects}:

function project_issue_project_load(&$node, $arg) {
  $project = db_fetch_object(db_query('SELECT * FROM {project_issue_projects} WHERE nid = %d', $node->nid));
  // If nothing loaded, bail immediately
  if (!$project) {
    return $node;
  }
  //assert($project);
  $node->components = unserialize($project->components);
  ...

Here return $node; has been used to match the later return in the function, although this the return value is ignored by project_issue_project_nodeapi(), which appears to be the only place it is called ($node is called by reference and modified in place). Perhaps some documentation for project_issue_project_nodeapi() could clarify whether it should return anything.

Best wishes,

Mark.

CommentFileSizeAuthor
project_issue.module.patch3.74 KBplumbley

Comments

plumbley’s picture

Status: Active » Needs review

(Set correct status. Again :-( )

dww’s picture

Status: Needs review » Fixed

Patch didn't cleanly apply, but I resolved the conflicts, reviewed, tested, and commited to HEAD, DRUPAL-4-7--2 and DRUPAL-4-7. A good step towards the D6 port. ;) Thanks.

Anonymous’s picture

Status: Fixed » Closed (fixed)