- Patch #809600 by Damien Tournoud, chx: stop using bit-wise operators for {file_managed()}.status.

6 files changed, 34 insertions, 46 deletions

diff --git a/includes/file.inc b/includes/file.inc
index ccdaa96a6..9fbecfed0 100644
--- a/includes/file.inc
+++ b/includes/file.inc
@@ -1090,17 +1090,15 @@ function file_unmanaged_delete_recursive($path) {
  *   Optional. A user id, specifying NULL returns the total space used by all
  *   non-temporary files.
  * @param $status
- *   Optional. File Status to return. Combine with a bitwise OR(|) to return
- *   multiple statuses. The default status is FILE_STATUS_PERMANENT.
+ *   Optional. The file status to consider. The default is to only
+ *   consider files in status FILE_STATUS_PERMANENT.
  *
  * @return
  *   An integer containing the number of bytes used.
  */
 function file_space_used($uid = NULL, $status = FILE_STATUS_PERMANENT) {
   $query = db_select('file_managed', 'f');
-  // Use separate placeholders for the status to avoid a bug in some versions
-  // of PHP. See http://drupal.org/node/352956.
-  $query->where('f.status & :status1 = :status2', array(':status1' => $status, ':status2' => $status));
+  $query->condition('f.status', $status);
   $query->addExpression('SUM(f.filesize)', 'filesize');
   if (!is_null($uid)) {
     $query->condition('f.uid', $uid);
@@ -1549,7 +1547,7 @@ function file_save_data($data, $destination = NULL, $replace = FILE_EXISTS_RENAM
     $file->filename = basename($uri);
     $file->filemime = file_get_mimetype($file->uri);
     $file->uid      = $user->uid;
-    $file->status  |= FILE_STATUS_PERMANENT;
+    $file->status   = FILE_STATUS_PERMANENT;
     // If we are replacing an existing file re-use its database record.
     if ($replace == FILE_EXISTS_REPLACE) {
       $existing_files = file_load_multiple(array(), array('uri' => $uri));
diff --git a/modules/file/file.field.inc b/modules/file/file.field.inc
index e0d452bd1..8184ae8bd 100644
--- a/modules/file/file.field.inc
+++ b/modules/file/file.field.inc
@@ -255,8 +255,8 @@ function file_field_presave($entity_type, $entity, $field, $instance, $langcode,
   // cleaned up.
   foreach ($items as $item) {
     $file = file_load($item['fid']);
-    if (($file->status & FILE_STATUS_PERMANENT) == 0) {
-      $file->status |= FILE_STATUS_PERMANENT;
+    if (!$file->status) {
+      $file->status = FILE_STATUS_PERMANENT;
       file_save($file);
     }
   }
diff --git a/modules/simpletest/tests/file.test b/modules/simpletest/tests/file.test
index 428b5a821..4cfdad807 100644
--- a/modules/simpletest/tests/file.test
+++ b/modules/simpletest/tests/file.test
@@ -297,44 +297,33 @@ class FileSpaceUsedTest extends FileTestCase {
     $file = array('uid' => 3, 'uri' => 'public://example4.txt', 'filesize' => 200, 'status' => FILE_STATUS_PERMANENT);
     drupal_write_record('file_managed', $file);
 
-    // Now create some with other statuses. These values were chosen arbitrarily
-    // for the sole purpose of testing that bitwise operators were used
-    // correctly on the field.
-    $file = array('uid' => 2, 'uri' => 'public://example5.txt', 'filesize' => 1, 'status' => 2 | 8);
+    // Now create some non-permanent files.
+    $file = array('uid' => 2, 'uri' => 'public://example5.txt', 'filesize' => 1, 'status' => 0);
     drupal_write_record('file_managed', $file);
-    $file = array('uid' => 3, 'uri' => 'public://example6.txt', 'filesize' => 3, 'status' => 2 | 4);
+    $file = array('uid' => 3, 'uri' => 'public://example6.txt', 'filesize' => 3, 'status' => 0);
     drupal_write_record('file_managed', $file);
   }
 
   /**
    * Test different users with the default status.
    */
-  function testUser() {
-    $this->assertEqual(file_space_used(2), 70, t("Found the size of the first user's files."));
-    $this->assertEqual(file_space_used(3), 300, t("Found the size of the second user's files."));
-    $this->assertEqual(file_space_used(), 370, t("Found the size of all user's files."));
-  }
-
-  /**
-   * Test the status fields
-   */
-  function testStatus() {
-    // Check selection with a single bit set.
-    $this->assertEqual(file_space_used(NULL, 2), 4, t("Found the size of all user's files with status 2."));
-    $this->assertEqual(file_space_used(NULL, 4), 3, t("Found the size of all user's files with status 4."));
-    // Check that the bitwise AND operator is used when selecting so that we
-    // only get files with the 2 AND 4 bits set.
-    $this->assertEqual(file_space_used(NULL, 2 | 4), 3, t("Found the size of all user's files with status 6."));
-  }
-
-  /**
-   * Test both the user and status.
-   */
-  function testUserAndStatus() {
-    $this->assertEqual(file_space_used(1, 8), 0, t("Found the size of the admin user's files with status 8."));
-    $this->assertEqual(file_space_used(2, 8), 1, t("Found the size of the first user's files with status 8."));
-    $this->assertEqual(file_space_used(2, 2), 1, t("Found the size of the first user's files with status 2."));
-    $this->assertEqual(file_space_used(3, 2), 3, t("Found the size of the second user's files with status 2."));
+  function testFileSpaceUsed() {
+    // Test different users with default status.
+    $this->assertEqual(file_space_used(2), 70);
+    $this->assertEqual(file_space_used(3), 300);
+    $this->assertEqual(file_space_used(), 370);
+
+    // Test the status fields
+    $this->assertEqual(file_space_used(NULL, 0), 4);
+    $this->assertEqual(file_space_used(NULL, FILE_STATUS_PERMANENT), 370);
+
+    // Test both the user and status.
+    $this->assertEqual(file_space_used(1, 0), 0);
+    $this->assertEqual(file_space_used(1, FILE_STATUS_PERMANENT), 0);
+    $this->assertEqual(file_space_used(2, 0), 1);
+    $this->assertEqual(file_space_used(2, FILE_STATUS_PERMANENT), 70);
+    $this->assertEqual(file_space_used(3, 0), 3);
+    $this->assertEqual(file_space_used(3, FILE_STATUS_PERMANENT), 300);
   }
 }
 
diff --git a/modules/system/system.install b/modules/system/system.install
index 9fd1b714a..cd2e9cb94 100644
--- a/modules/system/system.install
+++ b/modules/system/system.install
@@ -805,10 +805,11 @@ function system_schema() {
         'default' => 0,
       ),
       'status' => array(
-        'description' => 'A bitmapped field indicating the status of the file. The least significant bit indicates temporary (0) or permanent (1). Temporary files older than DRUPAL_MAXIMUM_TEMP_FILE_AGE will be removed during a cron run.',
+        'description' => 'A field indicating the status of the file. Two status are defined in core: temporary (0) and permanent (1). Temporary files older than DRUPAL_MAXIMUM_TEMP_FILE_AGE will be removed during a cron run.',
         'type' => 'int',
         'not null' => TRUE,
         'default' => 0,
+        'size' => 'tiny',
       ),
       'timestamp' => array(
         'description' => 'UNIX timestamp for when the file was added.',
@@ -2193,10 +2194,11 @@ function system_update_7034() {
         'default' => 0,
       ),
       'status' => array(
-        'description' => 'A bitmapped field indicating the status of the file the least sigifigant bit indicates temporary (1) or permanent (0). Temporary files older than DRUPAL_MAXIMUM_TEMP_FILE_AGE will be removed during a cron run.',
+        'description' => 'A field indicating the status of the file. Two status are defined in core: temporary (0) and permanent (1). Temporary files older than DRUPAL_MAXIMUM_TEMP_FILE_AGE will be removed during a cron run.',
         'type' => 'int',
         'not null' => TRUE,
         'default' => 0,
+        'size' => 'tiny',
       ),
       'timestamp' => array(
         'description' => 'UNIX timestamp for when the file was added.',
diff --git a/modules/system/system.module b/modules/system/system.module
index 033e0bdbe..f16927aa4 100644
--- a/modules/system/system.module
+++ b/modules/system/system.module
@@ -2794,9 +2794,8 @@ function system_cron() {
   // Remove temporary files that are older than DRUPAL_MAXIMUM_TEMP_FILE_AGE.
   // Use separate placeholders for the status to avoid a bug in some versions
   // of PHP. See http://drupal.org/node/352956.
-  $result = db_query('SELECT fid FROM {file_managed} WHERE status & :permanent1 <> :permanent2 AND timestamp < :timestamp', array(
-    ':permanent1' => FILE_STATUS_PERMANENT,
-    ':permanent2' => FILE_STATUS_PERMANENT,
+  $result = db_query('SELECT fid FROM {file_managed} WHERE status <> :permanent AND timestamp < :timestamp', array(
+    ':permanent' => FILE_STATUS_PERMANENT,
     ':timestamp' => REQUEST_TIME - DRUPAL_MAXIMUM_TEMP_FILE_AGE
   ));
   foreach ($result as $row) {
diff --git a/modules/user/user.module b/modules/user/user.module
index d5f97c99e..8e5e8f8d6 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -431,7 +431,7 @@ function user_save($account, $edit = array(), $category = 'account') {
         $picture = $edit['picture'];
         // If the picture is a temporary file move it to its final location and
         // make it permanent.
-        if (($picture->status & FILE_STATUS_PERMANENT) == 0) {
+        if (!$picture->status) {
           $info = image_get_info($picture->uri);
           $picture_directory =  variable_get('file_default_scheme', 'public') . '://' . variable_get('user_picture_path', 'pictures');
 
@@ -440,7 +440,7 @@ function user_save($account, $edit = array(), $category = 'account') {
           $destination = file_stream_wrapper_uri_normalize($picture_directory . '/picture-' . $account->uid . '-' . REQUEST_TIME . '.' . $info['extension']);
 
           if ($picture = file_move($picture, $destination, FILE_EXISTS_RENAME)) {
-            $picture->status |= FILE_STATUS_PERMANENT;
+            $picture->status = FILE_STATUS_PERMANENT;
             $edit['picture'] = file_save($picture);
           }
         }