Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 85fe2ccc authored by Jacky Cheung's avatar Jacky Cheung Committed by Marie Janssen
Browse files

Improve config file sync during configuration save. 

Add fsync() calls to force sync to disk while saving configuration
file.  It is necessary to do a sync on the temp file before rename,
and to sync again on its parent directory to ensure both the
file content and the directory are up-to-date.

Change-Id: I3a862ad59c8ae5beb8ea2c727eb5f275a2d5823e
parent cdff89bd

system/btif/src/btif_config.cc

+0 −2
Original line number Diff line number Diff line
@@ -480,12 +480,10 @@ static void btif_config_write(UNUSED_ATTR UINT16 event, UNUSED_ATTR char *p_para 


  pthread_mutex_lock(&lock);

  rename(CONFIG_FILE_PATH, CONFIG_BACKUP_PATH);

  sync();

  config_t *config_paired = config_new_clone(config);

  btif_config_remove_unpaired(config_paired);

  config_save(config_paired, CONFIG_FILE_PATH);

  config_free(config_paired);

  sync();

  pthread_mutex_unlock(&lock);

}

system/osi/src/config.c

+57 −8
Original line number Diff line number Diff line
@@ -23,6 +23,8 @@ 
#include <assert.h>

#include <ctype.h>

#include <errno.h>

#include <fcntl.h>

#include <libgen.h>

#include <stdio.h>

#include <stdlib.h>

#include <string.h>
@@ -278,12 +280,39 @@ bool config_save(const config_t *config, const char *filename) { 
  assert(filename != NULL);

  assert(*filename != '\0');



  char *temp_filename = osi_calloc(strlen(filename) + 5);

  // Steps to ensure content of config file gets to disk:

  //

  // 1) Open and write to temp file (e.g. bt_config.conf.new).

  // 2) Sync the temp file to disk with fsync().

  // 3) Rename temp file to actual config file (e.g. bt_config.conf).

  //    This ensures atomic update.

  // 4) Sync directory that has the conf file with fsync().

  //    This ensures directory entries are up-to-date.

  int dir_fd = -1;

  FILE *fp = NULL;



  // Build temp config file based on config file (e.g. bt_config.conf.new).

  static const char *temp_file_ext = ".new";

  const int filename_len = strlen(filename);

  const int temp_filename_len = filename_len + strlen(temp_file_ext) + 1;

  char *temp_filename = osi_calloc(temp_filename_len);

  snprintf(temp_filename, temp_filename_len, "%s%s", filename, temp_file_ext);



  // Extract directory from file path (e.g. /data/misc/bluedroid).

  char *temp_dirname = osi_strdup(filename);

  const char *directoryname = dirname(temp_dirname);

  if (!directoryname) {

    LOG_ERROR(LOG_TAG, "%s error extracting directory from '%s': %s", __func__, filename, strerror(errno));

    goto error;

  }



  strcpy(temp_filename, filename);

  strcat(temp_filename, ".new");

  dir_fd = open(directoryname, O_RDONLY);

  if (dir_fd < 0) {

    LOG_ERROR(LOG_TAG, "%s unable to open dir '%s': %s", __func__, directoryname, strerror(errno));

    goto error;

  }



  FILE *fp = fopen(temp_filename, "wt");

  fp = fopen(temp_filename, "wt");

  if (!fp) {

    LOG_ERROR(LOG_TAG, "%s unable to write file '%s': %s", __func__, temp_filename, strerror(errno));

    goto error;
@@ -313,9 +342,13 @@ bool config_save(const config_t *config, const char *filename) { 
    }

  }



  // Sync written temp file out to disk. fsync() is blocking until data makes it to disk.

  if (fsync(fileno(fp)) < 0) {

    LOG_WARN(LOG_TAG, "%s unable to fsync file '%s': %s", __func__, temp_filename, strerror(errno));

  }



  if (fclose(fp) == EOF) {

    LOG_ERROR(LOG_TAG, "%s unable to close file '%s': %s", __func__, temp_filename, strerror(errno));

    fp = NULL;

    goto error;

  }

  fp = NULL;
@@ -326,19 +359,35 @@ bool config_save(const config_t *config, const char *filename) { 
    goto error;

  }



  // Rename written temp file to the actual config file.

  if (rename(temp_filename, filename) == -1) {

    LOG_ERROR(LOG_TAG, "%s unable to commit file '%s': %s", __func__, filename, strerror(errno));

    goto error;

  }



  // This should ensure the directory is updated as well.

  if (fsync(dir_fd) < 0) {

    LOG_WARN(LOG_TAG, "%s unable to fsync dir '%s': %s", __func__, directoryname, strerror(errno));

  }



  if (close(dir_fd) < 0) {

    LOG_ERROR(LOG_TAG, "%s unable to close dir '%s': %s", __func__, directoryname, strerror(errno));

    goto error;

  }



  osi_free(temp_filename);

  osi_free(temp_dirname);

  return true;



error:;

  if (fp != NULL)

    fclose(fp);

error:

  // This indicates there is a write issue.  Unlink as partial data is not acceptable.

  unlink(temp_filename);

  if (fp)

    fclose(fp);

  if (dir_fd != -1)

    close(dir_fd);

  osi_free(temp_filename);

  osi_free(temp_dirname);

  return false;

}