Merge pull request #2218 from pi-hole/tweak/file_locking

 Increase compatibility with volumes on network shares

src/config/dnsmasq_config.c

@@ -14,8 +14,6 @@
 #include "log.h"
 // get_blocking_mode_str()
 #include "datastructure.h"
-// flock(), LOCK_SH
-#include <sys/file.h>
 // struct config
 #include "config/config.h"
 // JSON array functions
@@ -325,12 +323,7 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_
 	}
 
 	// Lock file, may block if the file is currently opened
-	if(flock(fileno(pihole_conf), LOCK_EX) != 0)
-	{
-		log_err("Cannot open "DNSMASQ_TEMP_CONF" in exclusive mode: %s", strerror(errno));
-		fclose(pihole_conf);
-		return false;
-	}
+	const bool locked = lock_file(pihole_conf, DNSMASQ_TEMP_CONF);
 
 	write_config_header(pihole_conf, "Dnsmasq config for Pi-hole's FTLDNS");
 	fputs("hostsdir="DNSMASQ_HOSTSDIR"\n", pihole_conf);
@@ -768,12 +761,8 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_
 	fflush(pihole_conf);
 
 	// Unlock file
-	if(flock(fileno(pihole_conf), LOCK_UN) != 0)
-	{
-		log_err("Cannot release lock on dnsmasq config file: %s", strerror(errno));
-		fclose(pihole_conf);
-		return false;
-	}
+	if(locked)
+		unlock_file(pihole_conf, DNSMASQ_TEMP_CONF);
 
 	// Close file
 	if(fclose(pihole_conf) != 0)
@@ -1037,12 +1026,7 @@ bool write_custom_list(void)
 	}
 
 	// Lock file, may block if the file is currently opened
-	if(flock(fileno(custom_list), LOCK_EX) != 0)
-	{
-		log_err("Cannot open "DNSMASQ_CUSTOM_LIST_LEGACY".tmp in exclusive mode: %s", strerror(errno));
-		fclose(custom_list);
-		return false;
-	}
+	const bool locked = lock_file(custom_list, DNSMASQ_CUSTOM_LIST_LEGACY".tmp");
 
 	write_config_header(custom_list, "Custom DNS entries (HOSTS file)");
 	fputc('\n', custom_list);
@@ -1067,12 +1051,8 @@ bool write_custom_list(void)
 		fputs("\n# There are currently no entries in this file\n", custom_list);
 
 	// Unlock file
-	if(flock(fileno(custom_list), LOCK_UN) != 0)
-	{
-		log_err("Cannot release lock on custom.list: %s", strerror(errno));
-		fclose(custom_list);
-		return false;
-	}
+	if(locked)
+		unlock_file(custom_list, DNSMASQ_CUSTOM_LIST_LEGACY".tmp");
 
 	// Close file
 	if(fclose(custom_list) != 0)

src/config/env.c

@@ -633,8 +633,9 @@ cJSON *read_forced_vars(const unsigned int version)
 	cJSON *env_vars = cJSON_CreateArray();
 
 	// Try to open default config file. Use fallback if not found
-	FILE *fp;
-	if((fp = openFTLtoml("r", version)) == NULL)
+	bool locked = false;
+	FILE *fp = openFTLtoml("r", version, &locked);
+	if(fp == NULL)
 	{
 		// Return empty cJSON array
 		return env_vars;
@@ -672,7 +673,7 @@ cJSON *read_forced_vars(const unsigned int version)
 	}
 
 	// Close file and release exclusive lock
-	closeFTLtoml(fp);
+	closeFTLtoml(fp, locked);
 
 	// Return cJSON array
 	return env_vars;

src/config/toml_helper.c

@@ -13,8 +13,6 @@
 #include "config/config.h"
 // get_refresh_hostnames_str()
 #include "datastructure.h"
-// flock(), LOCK_SH
-#include <sys/file.h>
 // fcntl(), O_ACCMODE, O_RDONLY
 #include <fcntl.h>
 // rotate_files()
@@ -29,7 +27,7 @@
 #include "files.h"
 
 // Open the TOML file for reading or writing
-FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *mode, const unsigned int version)
+FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *mode, const unsigned int version, bool *locked)
 {
 	// This should not happen, install a safeguard anyway to unveil
 	// possible future coding issues early on
@@ -69,15 +67,7 @@ FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *m
 	}
 
 	// Lock file, may block if the file is currently opened
-	if(flock(fileno(fp), LOCK_EX) != 0)
-	{
-		const int _e = errno;
-		log_err("Cannot open config file %s in exclusive mode (%s): %s",
-		        filename, mode, strerror(errno));
-		fclose(fp);
-		errno = _e;
-		return NULL;
-	}
+	*locked = lock_file(fp, filename);
 
 	// Log if we are using a backup file
 	if(version > 0)
@@ -88,14 +78,14 @@ FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *m
 }
 
 // Open the TOML file for reading or writing
-void closeFTLtoml(FILE *fp)
+void closeFTLtoml(FILE *fp, const bool locked)
 {
 	// Release file lock
-	const int fn = fileno(fp);
-	if(flock(fn, LOCK_UN) != 0)
-		log_err("Cannot release lock on FTL's config file: %s", strerror(errno));
+	if(locked)
+		unlock_file(fp, NULL);
 
 	// Get access mode
+	const int fn = fileno(fp);
 	const int mode = fcntl(fn, F_GETFL);
 	if (mode == -1)
 		log_err("Cannot get access mode for FTL's config file: %s", strerror(errno));

src/config/toml_helper.h

@@ -17,8 +17,8 @@
 #include "tomlc99/toml.h"
 
 void indentTOML(FILE *fp, const unsigned int indent);
-FILE *openFTLtoml(const char *mode, const unsigned int version) __attribute((malloc)) __attribute((nonnull(1)));
-void closeFTLtoml(FILE *fp);
+FILE *openFTLtoml(const char *mode, const unsigned int version, bool *locked) __attribute((malloc)) __attribute((nonnull(1)));
+void closeFTLtoml(FILE *fp, const bool locked);
 void print_comment(FILE *fp, const char *str, const char *intro, const unsigned int width, const unsigned int indent);
 void print_toml_allowed_values(cJSON *allowed_values, FILE *fp, const unsigned int width, const unsigned int indent);
 void writeTOMLvalue(FILE * fp, const int indent, const enum conf_type t, union conf_value *v);

src/config/toml_reader.c

@@ -228,16 +228,17 @@ bool readFTLtoml(struct config *oldconf, struct config *newconf,
 static toml_table_t *parseTOML(const unsigned int version)
 {
 	// Try to open default config file. Use fallback if not found
-	FILE *fp;
-	if((fp = openFTLtoml("r", version)) == NULL)
+	bool locked = false;
+	FILE *fp = openFTLtoml("r", version, &locked);
+	if(fp == NULL)
 		return NULL;
 
 	// Parse lines in the config file
 	char errbuf[200];
 	toml_table_t *conf = toml_parse_file(fp, errbuf, sizeof(errbuf));
 
 	// Close file and release exclusive lock
-	closeFTLtoml(fp);
+	closeFTLtoml(fp, locked);
 
 	// Check for errors
 	if(conf == NULL)

src/config/toml_writer.c

@@ -41,8 +41,9 @@ bool writeFTLtoml(const bool verbose)
 	}
 
 	// Try to open a temporary config file for writing
-	FILE *fp;
-	if((fp = openFTLtoml("w", 0)) == NULL)
+	bool locked = false;
+	FILE *fp = openFTLtoml("w", 0, &locked);
+	if(fp == NULL)
 		return false;
 
 	// Write header
@@ -163,7 +164,7 @@ bool writeFTLtoml(const bool verbose)
 	cJSON_Delete(env_vars);
 
 	// Close file and release exclusive lock
-	closeFTLtoml(fp);
+	closeFTLtoml(fp, locked);
 
 	// Move temporary file to the final location if it is different
 	// We skip the first 8 lines as they contain the header and will always

src/files.c

@@ -35,6 +35,8 @@
 #include <inttypes.h>
 //basename()
 #include <libgen.h>
+// flock(), LOCK_SH
+#include <sys/file.h>
 
 // chmod_file() changes the file mode bits of a given file (relative
 // to the directory file descriptor) according to mode. mode is an
@@ -863,3 +865,75 @@ enum verify_result verify_FTL(bool verbose)
 
 	return success ? VERIFY_OK : VERIFY_FAILED;
 }
+/**
+ * @brief Locks a file for exclusive access.
+ *
+ * This function attempts to acquire an exclusive lock on the file
+ * associated with the given file pointer.
+ *
+ * @param fp A pointer to the FILE object representing the file to be locked.
+ * @param filename The name of the file to be locked, used for logging purposes.
+ * @return true if the lock is successfully acquired, false otherwise.
+ */
+
+bool lock_file(FILE *fp, const char *filename)
+{
+	const int fd = fileno(fp);
+	if(fd < 0)
+	{
+		log_warn("Failed to get file descriptor for \"%s\": %s",
+		         filename, strerror(errno));
+		return false;
+	}
+
+	if(flock(fd, LOCK_EX) != 0)
+	{
+		// Backup errno
+		const int _e = errno;
+		log_warn("Cannot get exclusive lock for %s: %s",
+		         filename, strerror(errno));
+
+		// Restore errno
+		errno = _e;
+		return false;
+	}
+
+	return true;
+}
+
+/**
+ * @brief Unlocks a file by releasing the lock on the file descriptor.
+ *
+ * This function attempts to release the lock on the file associated with the
+ * given file pointer. If the file descriptor cannot be obtained or the lock
+ * cannot be released, an error message is logged and the function returns false.
+ *
+ * @param fp A pointer to the FILE object representing the file to unlock.
+ * @param filename A string representing the name of the file. This is used for
+ *                 logging purposes. If NULL, "<file>" is used in the log message.
+ * @return true if the lock was successfully released, false otherwise.
+ */
+bool unlock_file(FILE *fp, const char *filename)
+{
+	const int fd = fileno(fp);
+	if(fd < 0)
+	{
+		log_warn("Failed to get file descriptor for \"%s\": %s",
+		         filename ? filename : "<file>", strerror(errno));
+		return false;
+	}
+
+	if(flock(fd, LOCK_UN) != 0)
+	{
+		// Backup errno
+		const int _e = errno;
+		log_warn("Cannot release lock for %s: %s",
+		         filename ? filename : "<file>", strerror(errno));
+
+		// Restore errno
+		errno = _e;
+		return false;
+	}
+
+	return true;
+}

src/files.h

@@ -43,4 +43,7 @@ int parse_line(char *line, char **key, char **value);
 
 char *get_hwmon_target(const char *path) __attribute__((malloc));
 
+bool lock_file(FILE *fp, const char *filename);
+bool unlock_file(FILE *fp, const char *filename);
+
 #endif //FILE_H