- Patch #413732 by brianV: database code clean-up.

15 files changed, 120 insertions, 43 deletions

diff --git a/includes/database/database.inc b/includes/database/database.inc
index f07589b6f..f07e07373 100644
--- a/includes/database/database.inc
+++ b/includes/database/database.inc
@@ -516,7 +516,7 @@ abstract class DatabaseConnection extends PDO {
    *   A table prefix-parsed string for the sequence name.
    */
   public function makeSequenceName($table, $field) {
-    return $this->prefixTables('{'. $table .'}_'. $field .'_seq');
+    return $this->prefixTables('{' . $table . '}_' . $field . '_seq');
   }
 
   /**
@@ -594,7 +594,7 @@ abstract class DatabaseConnection extends PDO {
         else {
           $query_string = $query;
         }
-        throw new PDOException($query_string . " - \n" . print_r($args,1) . $e->getMessage());
+        throw new PDOException($query_string . " - \n" . print_r($args, 1) . $e->getMessage());
       }
       return NULL;
     }
@@ -1006,7 +1006,7 @@ abstract class DatabaseConnection extends PDO {
    * overridable lookup function. Database connections should define only
    * those operators they wish to be handled differently than the default.
    *
-   * @see DatabaseCondition::compile().
+   * @see DatabaseCondition::compile()
    * @param $operator
    *   The condition operator, such as "IN", "BETWEEN", etc. Case-sensitive.
    * @return
@@ -2191,8 +2191,9 @@ function db_drop_table(&$ret, $table) {
  *   table along with adding the field. The format is the same as a
  *   table specification but without the 'fields' element. If you are
  *   adding a type 'serial' field, you MUST specify at least one key
- *   or index including it in this array. @see db_change_field for more
+ *   or index including it in this array. See db_change_field() for more
  *   explanation why.
+ * @see db_change_field()
  */
 function db_add_field(&$ret, $table, $field, $spec, $keys_new = array()) {
   return Database::getConnection()->schema()->addField($ret, $table, $field, $spec, $keys_new);
@@ -2616,7 +2617,7 @@ function db_rewrite_sql($query, $primary_table = 'n', $primary_field = 'nid',  $
     if ($where) {
       $n = strlen($matches[1]);
       $second_part = substr($query, $n);
-      $first_part = substr($matches[1], 0, $n - 5) ." $join WHERE $where AND ( ";
+      $first_part = substr($matches[1], 0, $n - 5) . " $join WHERE $where AND ( ";
       foreach (array('GROUP', 'ORDER', 'LIMIT') as $needle) {
         $pos = strrpos($second_part, $needle);
         if ($pos !== FALSE) {
diff --git a/includes/database/mysql/install.inc b/includes/database/mysql/install.inc
index 5f15fa35d..cee986225 100644
--- a/includes/database/mysql/install.inc
+++ b/includes/database/mysql/install.inc
@@ -1,6 +1,12 @@
 <?php
 // $Id$
 
+/**
+ * @file
+ * Installation code for MySQL embedded database engine.
+ */
+
+
 // MySQL specific install functions
 
 class DatabaseInstaller_mysql extends DatabaseInstaller {
diff --git a/includes/database/mysql/query.inc b/includes/database/mysql/query.inc
index c9a3a51ce..b2dbae04c 100644
--- a/includes/database/mysql/query.inc
+++ b/includes/database/mysql/query.inc
@@ -6,6 +6,12 @@
  * @{
  */
 
+/**
+ * @file
+ * Query code for MySQL embedded database engine.
+ */
+
+
 class InsertQuery_mysql extends InsertQuery {
 
   public function execute() {
@@ -64,16 +70,16 @@ class InsertQuery_mysql extends InsertQuery {
 
         $new_placeholder = $max_placeholder + count($insert_values);
         for ($i = $max_placeholder; $i < $new_placeholder; ++$i) {
-          $placeholders[] = ':db_insert_placeholder_'. $i;
+          $placeholders[] = ':db_insert_placeholder_' . $i;
         }
         $max_placeholder = $new_placeholder;
-        $values[] = '('. implode(', ', $placeholders) .')';
+        $values[] = '(' . implode(', ', $placeholders) . ')';
       }
     }
     else {
       // If there are no values, then this is a default-only query. We still need to handle that.
       $placeholders = array_fill(0, count($this->defaultFields), 'default');
-      $values[] = '(' . implode(', ', $placeholders) .')';
+      $values[] = '(' . implode(', ', $placeholders) . ')';
     }
 
     $query .= implode(', ', $values);
diff --git a/includes/database/mysql/schema.inc b/includes/database/mysql/schema.inc
index b64e6040f..aa0905a35 100644
--- a/includes/database/mysql/schema.inc
+++ b/includes/database/mysql/schema.inc
@@ -224,7 +224,7 @@ class DatabaseSchema_mysql extends DatabaseSchema {
     }
     if (!empty($spec['unique keys'])) {
       foreach ($spec['unique keys'] as $key => $fields) {
-        $keys[] = 'UNIQUE KEY ' . $key .' ('. $this->createKeysSqlHelper($fields) . ')';
+        $keys[] = 'UNIQUE KEY ' . $key . ' (' . $this->createKeysSqlHelper($fields) . ')';
       }
     }
     if (!empty($spec['indexes'])) {
@@ -352,7 +352,7 @@ class DatabaseSchema_mysql extends DatabaseSchema {
 
     // Work around a bug in some versions of PDO, see http://bugs.php.net/bug.php?id=41125
     $comment = str_replace("'", '’', $comment);
- 
+
     // Truncate comment to maximum comment length.
     if (isset($length)) {
       // Add table prefixes before truncating.
diff --git a/includes/database/pgsql/database.inc b/includes/database/pgsql/database.inc
index c9dd63a06..881ea3d97 100644
--- a/includes/database/pgsql/database.inc
+++ b/includes/database/pgsql/database.inc
@@ -72,7 +72,7 @@ class DatabaseConnection_pgsql extends DatabaseConnection {
         else {
           $query_string = $query;
         }
-        throw new PDOException($query_string . " - \n" . print_r($args,1) . $e->getMessage());
+        throw new PDOException($query_string . " - \n" . print_r($args, 1) . $e->getMessage());
       }
       return NULL;
     }
diff --git a/includes/database/pgsql/install.inc b/includes/database/pgsql/install.inc
index 921e21402..b301cb672 100644
--- a/includes/database/pgsql/install.inc
+++ b/includes/database/pgsql/install.inc
@@ -1,6 +1,12 @@
 <?php
 // $Id$
 
+/**
+ * @file
+ * Install functions for PostgreSQL embedded database engine.
+ */
+
+
 // PostgreSQL specific install functions
 
 class DatabaseInstaller_pgsql extends DatabaseInstaller {
diff --git a/includes/database/pgsql/query.inc b/includes/database/pgsql/query.inc
index a0f97b1b4..f3a4932ea 100644
--- a/includes/database/pgsql/query.inc
+++ b/includes/database/pgsql/query.inc
@@ -6,6 +6,12 @@
  * @{
  */
 
+/**
+ * @file
+ * Query code for PostgreSQL embedded database engine.
+ */
+
+
 class InsertQuery_pgsql extends InsertQuery {
 
   public function __construct($connection, $table, array $options = array()) {
@@ -51,7 +57,7 @@ class InsertQuery_pgsql extends InsertQuery {
           ++$blob_count;
         }
         else {
-          $stmt->bindParam(':db_insert_placeholder_'. $max_placeholder++, $insert_values[$idx]);
+          $stmt->bindParam(':db_insert_placeholder_' . $max_placeholder++, $insert_values[$idx]);
         }
       }
     }
@@ -101,7 +107,7 @@ class InsertQuery_pgsql extends InsertQuery {
     else {
       // If there are no values, then this is a default-only query. We still need to handle that.
       $placeholders = array_fill(0, count($this->defaultFields), 'default');
-      $values[] = '(' . implode(', ', $placeholders) .')';
+      $values[] = '(' . implode(', ', $placeholders) . ')';
     }
 
     $query .= implode(', ', $values);
diff --git a/includes/database/pgsql/schema.inc b/includes/database/pgsql/schema.inc
index c4dda4c03..08b04b938 100644
--- a/includes/database/pgsql/schema.inc
+++ b/includes/database/pgsql/schema.inc
@@ -19,7 +19,7 @@ class DatabaseSchema_pgsql extends DatabaseSchema {
    * This is collected by DatabaseConnection_pgsql->queryTableInformation(),
    * by introspecting the database.
    *
-   * @see DatabaseConnection_pgsql->queryTableInformation().
+   * @see DatabaseConnection_pgsql->queryTableInformation()
    * @var array
    */
   protected $tableInformation = array();
@@ -46,7 +46,7 @@ class DatabaseSchema_pgsql extends DatabaseSchema {
 
     if (!isset($this->tableInformation[$key])) {
       // Split the key into schema and table for querying.
-      list($schema,$table_name) = explode('.', $key);
+      list($schema, $table_name) = explode('.', $key);
       $table_information = (object) array(
         'blob_fields' => array(),
         'sequences' => array(),
@@ -56,7 +56,7 @@ class DatabaseSchema_pgsql extends DatabaseSchema {
         if ($column->data_type == 'bytea') {
           $table_information->blob_fields[$column->column_name] = TRUE;
         }
-        else if (preg_match("/nextval\('([^']+)'/", $column->column_default, $matches)) {
+        elseif (preg_match("/nextval\('([^']+)'/", $column->column_default, $matches)) {
           // We must know of any sequences in the table structure to help us
           // return the last insert id. If there is more than 1 sequences the
           // first one (index 0 of the sequences array) will be used.
@@ -311,8 +311,10 @@ class DatabaseSchema_pgsql extends DatabaseSchema {
    *   table along with adding the field. The format is the same as a
    *   table specification but without the 'fields' element. If you are
    *   adding a type 'serial' field, you MUST specify at least one key
-   *   or index including it in this array. @see db_change_field for more
+   *   or index including it in this array. See db_change_field() for more
    *   explanation why.
+   *
+   * @see db_change_field()
    */
   public function addField(&$ret, $table, $field, $spec, $new_keys = array()) {
     $fixnull = FALSE;
diff --git a/includes/database/prefetch.inc b/includes/database/prefetch.inc
index dd8f96fd7..e4db32464 100644
--- a/includes/database/prefetch.inc
+++ b/includes/database/prefetch.inc
@@ -1,5 +1,5 @@
 <?php
-// $Id $
+// $Id$
 
 /**
  * @file
@@ -89,7 +89,7 @@ class DatabaseStatementPrefetch implements Iterator, DatabaseStatementInterface
 
   /**
    * Holds the current fetch style (which will be used by the next fetch).
-   * @see PDOStatement::fetch.
+   * @see PDOStatement::fetch()
    *
    * @var int
    */
@@ -177,7 +177,8 @@ class DatabaseStatementPrefetch implements Iterator, DatabaseStatementInterface
     $this->rowCount = $statement->rowCount();
     $this->data = $statement->fetchAll(PDO::FETCH_ASSOC);
     // Destroy the statement as soon as possible.
-    // @see DatabaseConnection_sqlite::PDOPrepare for explanation.
+    // See DatabaseConnection_sqlite::PDOPrepare() for explanation.
+    // @see DatabaseConnection_sqlite::PDOPrepare()
     unset($statement);
 
     $this->resultRowCount = count($this->data);
@@ -236,7 +237,7 @@ class DatabaseStatementPrefetch implements Iterator, DatabaseStatementInterface
   }
 
   /**
-   * @see PDOStatement::setFetchMode.
+   * @see PDOStatement::setFetchMode()
    */
   public function setFetchMode($fetchStyle, $a2 = NULL, $a3 = NULL) {
     $this->defaultFetchStyle = $fetchStyle;
diff --git a/includes/database/query.inc b/includes/database/query.inc
index 8267d27c6..8c7281820 100644
--- a/includes/database/query.inc
+++ b/includes/database/query.inc
@@ -1,4 +1,5 @@
 <?php
+// $Id$
 
 /**
  * @ingroup database
@@ -6,6 +7,11 @@
  */
 
 /**
+ * @file
+ * Non-specific Database query code. Used by all engines.
+ */
+
+/**
  * Interface for a conditional clause in a query.
  */
 interface QueryConditionInterface {
@@ -462,7 +468,7 @@ class InsertQuery extends Query {
     $placeholders = array_pad($placeholders, count($this->defaultFields), 'default');
     $placeholders = array_pad($placeholders, count($this->insertFields), '?');
 
-    return 'INSERT INTO {'. $this->table .'} ('. implode(', ', $insert_fields) .') VALUES ('. implode(', ', $placeholders) .')';
+    return 'INSERT INTO {' . $this->table . '} (' . implode(', ', $insert_fields) . ') VALUES (' . implode(', ', $placeholders) . ')';
   }
 }
 
diff --git a/includes/database/sqlite/database.inc b/includes/database/sqlite/database.inc
index e872f5c3c..abfe42c16 100644
--- a/includes/database/sqlite/database.inc
+++ b/includes/database/sqlite/database.inc
@@ -25,7 +25,7 @@ class DatabaseConnection_sqlite extends DatabaseConnection {
     // This driver defaults to transaction support, except if explicitly passed FALSE.
     $this->transactionSupport = !isset($connection_options['transactions']) || $connection_options['transactions'] !== FALSE;
 
-    parent::__construct('sqlite:'. $connection_options['database'], '', '', array(
+    parent::__construct('sqlite:' . $connection_options['database'], '', '', array(
       // Force column names to lower case.
       PDO::ATTR_CASE => PDO::CASE_LOWER,
     ));
@@ -162,8 +162,10 @@ class DatabaseConnection_sqlite extends DatabaseConnection {
 /**
  * Specific SQLite implementation of DatabaseConnection.
  *
- * @see DatabaseConnection_sqlite::PDOPrepare for reasons why we must prefetch
+ * See DatabaseConnection_sqlite::PDOPrepare() for reasons why we must prefetch
  * the data instead of using PDOStatement.
+ *
+ * @see DatabaseConnection_sqlite::PDOPrepare()
  */
 class DatabaseStatement_sqlite extends DatabaseStatementPrefetch implements Iterator, DatabaseStatementInterface {
 
diff --git a/includes/database/sqlite/query.inc b/includes/database/sqlite/query.inc
index aff32802f..ab6e2da50 100644
--- a/includes/database/sqlite/query.inc
+++ b/includes/database/sqlite/query.inc
@@ -1,5 +1,10 @@
 <?php
-// $Id $
+// $Id$
+
+/**
+ * @file
+ * Query code for SQLite embedded database engine.
+ */
 
 /**
  * @ingroup database
@@ -27,14 +32,14 @@ class InsertQuery_sqlite extends InsertQuery {
       return parent::execute();
     }
     else {
-      return $this->connection->query('INSERT INTO {'. $this->table .'} DEFAULT VALUES', array(), $this->queryOptions);
+      return $this->connection->query('INSERT INTO {' . $this->table . '} DEFAULT VALUES', array(), $this->queryOptions);
     }
   }
 
   public function __toString() {
     // Produce as many generic placeholders as necessary.
     $placeholders = array_fill(0, count($this->insertFields), '?');
-    return 'INSERT INTO {'. $this->table .'} ('. implode(', ', $this->insertFields) .') VALUES ('. implode(', ', $placeholders) .')';
+    return 'INSERT INTO {' . $this->table . '} (' . implode(', ', $this->insertFields) . ') VALUES (' . implode(', ', $placeholders) . ')';
   }
 
 }
@@ -87,7 +92,7 @@ class UpdateQuery_sqlite extends UpdateQuery {
         // The IS NULL operator is badly managed by DatabaseCondition.
         $condition->where($field . ' IS NULL');
       }
-      else if (is_null($data)) {
+      elseif (is_null($data)) {
         // The field will be set to NULL.
         // The IS NULL operator is badly managed by DatabaseCondition.
         $condition->where($field . ' IS NOT NULL');
diff --git a/includes/database/sqlite/schema.inc b/includes/database/sqlite/schema.inc
index 3014aeb7d..137592bbf 100644
--- a/includes/database/sqlite/schema.inc
+++ b/includes/database/sqlite/schema.inc
@@ -66,7 +66,7 @@ class DatabaseSchema_sqlite extends DatabaseSchema {
     // Add the SQL statement for each field.
     foreach ($schema['fields'] as $name => $field) {
       if ($field['type'] == 'serial') {
-        if (isset($schema['primary key']) && ($key = array_search($name, $schema['primary key'])) !== false) {
+        if (isset($schema['primary key']) && ($key = array_search($name, $schema['primary key'])) !== FALSE) {
           unset($schema['primary key'][$key]);
         }
       }
@@ -294,7 +294,8 @@ class DatabaseSchema_sqlite extends DatabaseSchema {
    * @param $table
    *   Name of the table.
    * @return
-   *   An array representing the schema, @see drupal_get_schema.
+   *   An array representing the schema, from drupal_get_schema().
+   * @see drupal_get_schema()
    */
   protected function introspectSchema($table) {
     $mapped_fields = array_flip($this->getFieldTypeMap());
diff --git a/includes/file.inc b/includes/file.inc
index 75f2dc997..e61574501 100644
--- a/includes/file.inc
+++ b/includes/file.inc
@@ -157,7 +157,7 @@ function file_check_directory(&$directory, $mode = 0, $form_item = NULL) {
   // Check if directory exists.
   if (!is_dir($directory)) {
     if (($mode & FILE_CREATE_DIRECTORY) && @mkdir($directory)) {
-      @chmod($directory, 0775); // Necessary for non-webserver users.
+      drupal_chmod($directory);
     }
     else {
       if ($form_item) {
@@ -172,7 +172,7 @@ function file_check_directory(&$directory, $mode = 0, $form_item = NULL) {
   if (!is_writable($directory)) {
     // If not able to modify permissions, or if able to, but chmod
     // fails, return false.
-    if (!$mode || (($mode & FILE_MODIFY_PERMISSIONS) && !@chmod($directory, 0775))) {
+    if (!$mode || (($mode & FILE_MODIFY_PERMISSIONS) && !drupal_chmod($directory))) {
       if ($form_item) {
         form_set_error($form_item, t('The directory %directory is not writable', array('%directory' => $directory)));
         watchdog('file system', 'The directory %directory is not writable, because it does not have the correct permissions set.', array('%directory' => $directory), WATCHDOG_ERROR);
@@ -183,9 +183,8 @@ function file_check_directory(&$directory, $mode = 0, $form_item = NULL) {
 
   if ((file_directory_path() == $directory || file_directory_temp() == $directory) && !is_file("$directory/.htaccess")) {
     $htaccess_lines = "SetHandler Drupal_Security_Do_Not_Remove_See_SA_2006_006\nOptions None\nOptions +FollowSymLinks";
-    if (($fp = fopen("$directory/.htaccess", 'w')) && fputs($fp, $htaccess_lines)) {
-      fclose($fp);
-      chmod($directory . '/.htaccess', 0664);
+    if (file_put_contents("$directory/.htaccess", $htaccess_lines)) {
+      drupal_chmod("$directory/.htaccess");
     }
     else {
       $variables = array('%directory' => $directory, '!htaccess' => '<br />' . nl2br(check_plain($htaccess_lines)));
@@ -483,11 +482,8 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST
     return FALSE;
   }
 
-  // Give everyone read access so that FTP'd users or
-  // non-webserver users can see/read these files,
-  // and give group write permissions so group members
-  // can alter files uploaded by the webserver.
-  @chmod($destination, 0664);
+  // Set the permissions on the new file.
+  drupal_chmod($destination);
 
   return $destination;
 }
@@ -996,6 +992,9 @@ function file_save_upload($source, $validators = array(), $destination = FALSE,
     return FALSE;
   }
 
+  // Set the permissions on the new file.
+  drupal_chmod($file->filepath);
+
   // If we are replacing an existing file re-use its database record.
   if ($replace == FILE_EXISTS_REPLACE) {
     $existing_files = file_load_multiple(array(), array('filepath' => $file->filepath));
@@ -1889,6 +1888,43 @@ function file_get_mimetype($filename, $mapping = NULL) {
 
   return 'application/octet-stream';
 }
+
 /**
- * @} End of "defgroup file".
+ * Set the permissions on a file or directory.
+ *
+ * This function will use the 'file_chmod_directory' and 'file_chmod_file'
+ * variables for the default modes for directories and uploaded/generated files.
+ * By default these will give everyone read access so that users accessing the
+ * files with a user account without the webserver group (e.g. via FTP) can read
+ * these files, and give group write permissions so webserver group members
+ * (e.g. a vhost account) can alter files uploaded and owned by the webserver.
+ *
+ * @param $path
+ *   String containing the path to a file or directory.
+ * @param $mode
+ *   Integer value for the permissions. Consult PHP chmod() documentation for
+ *   more information.
+ * @return
+ *   TRUE for success, FALSE in the event of an error.
  */
+function drupal_chmod($path, $mode = NULL) {
+  if (!isset($mode)) {
+    if (is_dir($path)) {
+      $mode = variable_get('file_chmod_directory', 0775);
+    }
+    else {
+      $mode = variable_get('file_chmod_file', 0664);
+    }
+  }
+
+  if (@chmod($path, $mode)) {
+    return TRUE;
+  }
+
+  watchdog('file', 'The file permissions could not be set on %path.', array('%path' => $path), WATCHDOG_ERROR);
+  return FALSE;
+}
+
+/**
+ * @} End of "defgroup file".
+ */
\ No newline at end of file
diff --git a/includes/image.inc b/includes/image.inc
index df31990ac..e425d1251 100644
--- a/includes/image.inc
+++ b/includes/image.inc
@@ -376,10 +376,9 @@ function image_save(stdClass $image, $destination = NULL) {
     clearstatcache();
     $image->info = image_get_info($destination);
 
-    if (@chmod($destination, 0664)) {
+    if (drupal_chmod($destination)) {
       return $return;
     }
-    watchdog('image', 'Could not set permissions on destination file: %file', array('%file' => $destination));
   }
   return FALSE;
 }