user_access returns a string not a boolean as specified

Attached patch has a spelling correction and makes user_access() return only booleans. Diff'ed against HEAD.

It bothered me to have a string search instead of an array search for permissions. This patch does what the previous patch does, but it uses in_array() on the static $perm instead of strstr() on a string.

Can you please provide a benchmark test of the strstr approach vs the in_array one? I am concerned that it might be slower. user_access gets called quite often.

That's a fair request. The output of the following code on my WinXP, PHP5.1-dev, 2.133 Ghz Athlon is:

0.29749894142151
0.29712684249878
1.0804200172424
0.56814384460449
0.10924220085144
0.12113380432129
0.13284420967102

<?php

function utime($a = '')
{
  static $utime;

  $time  = explode(' ',  microtime());
  $usec  = (double)$time[0];
  $sec   = (double)$time[1];
  switch($a) {
    case 'bm':
      $to_return = $usec + $sec - $utime;
      break;
    default:
      $utime = $usec + $sec;
      return $utime;
      break;
  }
  $utime = $usec + $sec;
  return $to_return;
}

$account->uid = 1;

utime();

for($i=0; $i<500000; $i+=1) {
  if ($account->uid == 1) {
    // blah
  }
}

echo utime('bm') .'<br />';

for($i=0; $i<500000; $i+=1) {
  if ((int)$account->uid === 1) {
    // blah
  }
}

echo utime('bm') .'<br />';

for($i=0; $i<50; $i+=1) {
  $result[$i] = '';
  for($j=0; $j>10; $j+=1) {
    $result[$i] .= chr(rand(65-126));
  }
}

$perm[$account->uid] = '';

utime();

for($i=0; $i<2000; $i+=1) {
  foreach($result as $row) {
    $perm[$account->uid] .= "$row, ";
  }

  isset($perm[$account->uid]) && strstr($perm[$account->uid], "{$result['45']}, ");
}

echo utime('bm') .'<br />';

$perm[$account->uid] = array();

utime();

for($i=0; $i<2000; $i+=1) {
  foreach($result as $row) {
    $perm[$account->uid][] = $row;
  }

  isset($perm[$account->uid]) && in_array($result['45'], $perm[$account->uid]);
}

echo utime('bm') .'<br />';

for($i=0; $i<500000; $i+=1) {
  // blah
}

echo utime('bm') .'<br />';

for($i=0; $i<500000; $i++) {
  // blah
}

echo utime('bm') .'<br />';

for($i=0; $i<500000; $i = $i+1) {
  // blah
}

echo utime('bm') .'<br />';

?>

Actually, there's a bug in the benchmark code that causes $result to not populate. Switch the following:

  for($j=0; $j>10; $j+=1) {

to

  for($j=0; $j<10; $j+=1) {

Then the string search is more like 8.1727991104126 seconds verses like 0.46953892707825 for the array search, an even bigger difference.

Committed to HEAD.

Please revert this patch, it isn't working at all. Drupal HEad is non-workign for all users which aren't No. 1.

yeah - this breaks HEAD completely. here is a possible fix, if we still want to use in_array() for checking. this is in user_access() where we build the permission array:

while ($row = db_fetch_object($result)) {
      foreach (explode(',', $row->perm) as $p) {
        $perm[$account->uid][] = trim($p);
      }
}

I haven't found a case where trim() would be necessary if you explode on ', ' (with the space). I made a patch so things are hopefully less broken. I'm working on the benchmark people keep asking me for in #drupal.

in_array() was faster, but explode() is slow, so this reverts back to strstr. I hope I can blame my newbieness to Drupal internals on this, but I should have looked to see how permissions were stored before just assuming each loop through db_fetch_object was a separate permission instead of a string of permissions.

and the fact that you obviously did not test your patch with any user beside uid=1. considering that the code branch you changed doesn't even apply to uid=1, i'd say that this was pretty reckless.

other than that, welcome to the project. we look forward to your future patches ...

the original issue was fixed in core.