Duplicate static variable [#1359492]

Was browsing the code and noticed that entity_ui_controller creates two static variables with drupal_static(__FUNCTION__) so it's just going to be the same variable with two names.

Comment	File	Size	Author
#3	entity-duplicate-static-1359492-3.patch	1.48 KB	loganfsmyth
#3
#1	entity-duplicate-static-1359492-1.patch	1.13 KB	loganfsmyth
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

File	Size
entity-duplicate-static-1359492-1.patch	1.13 KB

Comment #2

fago

German

Vienna

CreditAttribution: fago commented 27 December 2011 at 16:19

Status:

Needs review

» Needs work

Well, the serve a slightly different purpose. $all is only set if the info is loaded completely. With your patch the function might return incomplete results if all controllers are requested.

This weird logic is used to avoid loading all ui-controllers even if only one is requested.
Clean-up and/or docs to clarify that are of course highly welcome.

Comment #3

loganfsmyth CreditAttribution: loganfsmyth commented 28 December 2011 at 01:17

File	Size
entity-duplicate-static-1359492-3.patch	1.48 KB

@fago I see what you are saying, and after reviewing the function further, I think it simply doesn't work as intended. Currently it has the very problem you mention. http://drupalcode.org/project/entity.git/blob/refs/heads/7.x-1.x:/entity...

Currently the function has this in it:

$static = &drupal_static(__FUNCTION__);
$all = &drupal_static(__FUNCTION__);

So $static and $all will both be references to the same value. Also, $all is only ever set, but never read.

I propose a new implementation that uses one static and addresses the initialization problem you mentioned.

function entity_ui_controller($type = NULL) {                                                                                                                      
  $static = &drupal_static(__FUNCTION__);

  if (!isset($type)) {
    // Invoke the function for each type to ensure we have fully populated the
    // static variable $static.
    foreach (entity_get_info() as $entity_type => $info) {
      entity_ui_controller($entity_type);
    }    
    return array_filter($static);
  }

  if (!isset($static[$type])) {
    $info = entity_get_info($type);
    $class = isset($info['admin ui']['controller class']) ? $info['admin ui']['controller class'] : 'EntityDefaultUIController';
    $static[$type] = (isset($info['admin ui']['path']) && $class) ? new $class($type, $info) : FALSE;
  }

  return $static[$type];
}