Some operations, such as open_append and sync, need an exclusive lock
across a longer period than a single compress/decompress. Remove it
from the execute_command and pull into the outer callers. Store lock
information inside compress_info.
Sync and check_mailbox need more fixes, which will be addressed in
subsequent patches.
# HG changeset patch
# User Kevin McCarthy <ke...@8t8.us>
# Date 1478734909 28800
# Wed Nov 09 15:41:49 2016 -0800
# Node ID a49e229bbf3380fc6dc6e6e4cfb1072ebb512b48
# Parent 7186fdb54bdd233846c47818b264ff753ed3fdbd
Compress: pull the lock/unlock operations into the open,close,sync operations.
Some operations, such as open_append and sync, need an exclusive lock
across a longer period than a single compress/decompress. Remove it
from the execute_command and pull into the outer callers. Store lock
information inside compress_info.
Sync and check_mailbox need more fixes, which will be addressed in
subsequent patches.
diff --git a/compress.c b/compress.c
--- a/compress.c
+++ b/compress.c
@@ -44,74 +44,96 @@
*/
typedef struct
{
const char *append; /* append-hook command */
const char *close; /* close-hook command */
const char *open; /* open-hook command */
off_t size; /* size of the compressed file */
struct mx_ops *child_ops; /* callbacks of de-compressed file */
+ int locked; /* if realpath is locked */
+ FILE *lockfp; /* fp used for locking */
} COMPRESS_INFO;
/**
- * lock_mailbox - Try to lock a mailbox (exclusively)
+ * lock_realpath - Try to lock the ctx->realpath
* @ctx: Mailbox to lock
- * @fp: File pointer to the mailbox file
* @excl: Lock exclusively?
*
* Try to (exclusively) lock the mailbox. If we succeed, then we mark the
* mailbox as locked. If we fail, but we didn't want exclusive rights, then
* the mailbox will be marked readonly.
*
* Returns:
* 1: Success (locked or readonly)
* 0: Error (can't lock the file)
*/
static int
-lock_mailbox (CONTEXT *ctx, FILE *fp, int excl)
+lock_realpath (CONTEXT *ctx, int excl)
{
- if (!ctx || !fp)
+ if (!ctx)
return 0;
- int r = mx_lock_file (ctx->realpath, fileno (fp), excl, 1, 1);
+ COMPRESS_INFO *ci = ctx->compress_info;
+ if (!ci)
+ return 0;
+
+ if (ci->locked)
+ return 1;
+
+ if (excl)
+ ci->lockfp = fopen (ctx->realpath, "a");
+ else
+ ci->lockfp = fopen (ctx->realpath, "r");
+ if (!ci->lockfp)
+ {
+ mutt_perror (ctx->realpath);
+ return 0;
+ }
+
+ int r = mx_lock_file (ctx->realpath, fileno (ci->lockfp), excl, 1, 1);
if (r == 0)
- {
- ctx->locked = 1;
- }
+ ci->locked = 1;
else if (excl == 0)
{
+ safe_fclose (&ci->lockfp);
ctx->readonly = 1;
return 1;
}
return (r == 0);
}
/**
- * unlock_mailbox - Unlock a mailbox
+ * unlock_realpath - Unlock the ctx->realpath
* @ctx: Mailbox to unlock
- * @fp: File pointer to mailbox file
*
* Unlock a mailbox previously locked by lock_mailbox().
*/
static void
-unlock_mailbox (CONTEXT *ctx, FILE *fp)
+unlock_realpath (CONTEXT *ctx)
{
- if (!ctx || !fp)
+ if (!ctx)
return;
- if (!ctx->locked)
+ COMPRESS_INFO *ci = ctx->compress_info;
+ if (!ci)
return;
- fflush (fp);
+ if (!ci->locked)
+ return;
- mx_unlock_file (ctx->realpath, fileno (fp), 1);
- ctx->locked = 0;
+ fflush (ci->lockfp);
+
+ mx_unlock_file (ctx->realpath, fileno (ci->lockfp), 1);
+
+ ci->locked = 0;
+ safe_fclose (&ci->lockfp);
}
/**
* setup_paths - Set the mailbox paths
* @ctx: Mailbox to modify
*
* Save the compressed filename in ctx->realpath.
* Create a temporary filename and put its name in ctx->path.
@@ -265,16 +287,18 @@
if (!ctx || !ctx->compress_info)
return;
ci = ctx->compress_info;
FREE (&ci->open);
FREE (&ci->close);
FREE (&ci->append);
+ unlock_realpath (ctx);
+
FREE (&ctx->compress_info);
}
/**
* cb_format_str - Expand the filenames in the command string
* @dest: Buffer in which to save string
* @destlen: Buffer length
* @col: Starting column, UNUSED
@@ -340,78 +364,51 @@
mutt_FormatString (buf, buflen, 0, buflen, cmd, cb_format_str, (unsigned long) ctx, 0);
}
/**
* execute_command - Run a system command
* @ctx: Mailbox to work with
* @command: Command string to execute
- * @create_file: Should the tmp file be created?
* @progress: Message to show the user
*
* Run the supplied command, taking care of all the Mutt requirements,
* such as locking files and blocking signals.
*
* Returns:
* 1: Success
* 0: Failure
*/
static int
-execute_command (CONTEXT *ctx, const char *command, int create_file, const char *progress)
+execute_command (CONTEXT *ctx, const char *command, const char *progress)
{
- FILE *fp;
- int rc = 0;
- int unlock = 0;
+ int rc = 1;
char sys_cmd[HUGE_STRING];
if (!ctx || !command || !progress)
return 0;
if (!ctx->quiet)
mutt_message (progress, ctx->realpath);
- if (create_file)
- fp = fopen (ctx->realpath, "a");
- else
- fp = fopen (ctx->realpath, "r");
- if (!fp)
- {
- mutt_perror (ctx->realpath);
- return 0;
- }
-
mutt_block_signals();
- /* If we're creating the file, lock it exclusively */
- if (!lock_mailbox (ctx, fp, create_file))
- {
- mutt_error (_("Unable to lock mailbox!"));
- goto cleanup;
- }
-
- unlock = 1;
endwin();
fflush (stdout);
expand_command_str (ctx, command, sys_cmd, sizeof (sys_cmd));
if (mutt_system (sys_cmd) != 0)
{
+ rc = 0;
mutt_any_key_to_continue (NULL);
mutt_error (_("Error executing: %s\n"), sys_cmd);
- goto cleanup;
}
- rc = 1;
-
-cleanup:
- if (unlock)
- unlock_mailbox (ctx, fp);
mutt_unblock_signals();
- safe_fclose (&fp);
return rc;
}
/**
* open_mailbox - Open a compressed mailbox
* @ctx: Mailbox to open
*
@@ -433,20 +430,28 @@
/* If there's no close-hook, or the file isn't writable */
if (!ci->close || (access (ctx->path, W_OK) != 0))
ctx->readonly = 1;
if (setup_paths (ctx) != 0)
goto or_fail;
store_size (ctx);
- int rc = execute_command (ctx, ci->open, 0, _("Decompressing %s"));
+ if (!lock_realpath (ctx, 0))
+ {
+ mutt_error (_("Unable to lock mailbox!"));
+ goto or_fail;
+ }
+
+ int rc = execute_command (ctx, ci->open, _("Decompressing %s"));
if (rc == 0)
goto or_fail;
+ unlock_realpath (ctx);
+
ctx->magic = mx_get_magic (ctx->path);
if (ctx->magic == 0)
{
mutt_error (_("Can't identify the contents of the compressed file"));
goto or_fail;
}
ci->child_ops = mx_get_ops (ctx->magic);
@@ -506,20 +511,28 @@
ctx->mx_ops = &mx_comp_ops;
ci->child_ops = mx_get_ops (ctx->magic);
if (!ci->child_ops)
{
mutt_error (_("Can't find mailbox ops for mailbox type %d"), ctx->magic);
goto oa_fail2;
}
+ /* Lock the realpath for the duration of the append.
+ * It will be unlocked in the close */
+ if (!lock_realpath (ctx, 1))
+ {
+ mutt_error (_("Unable to lock mailbox!"));
+ goto oa_fail2;
+ }
+
/* Open the existing mailbox, unless we are appending */
- if (!ci->append && (access (ctx->realpath, F_OK) == 0))
+ if (!ci->append && (get_size (ctx->realpath) > 0))
{
- int rc = execute_command (ctx, ci->open, 0, _("Decompressing %s"));
+ int rc = execute_command (ctx, ci->open, _("Decompressing %s"));
if (rc == 0)
{
mutt_error (_("Compress command failed: %s"), ci->open);
goto oa_fail2;
}
}
if (ci->child_ops->open_append (ctx, flags) != 0)
@@ -590,25 +603,27 @@
msg = _("Compressed-appending to %s...");
}
else
{
append = ci->close;
msg = _("Compressing %s...");
}
- int rc = execute_command (ctx, append, 1, msg);
+ int rc = execute_command (ctx, append, msg);
if (rc == 0)
{
mutt_any_key_to_continue (NULL);
mutt_error (_(" %s: Error compressing mailbox! Uncompressed one kept!\n"), ctx->path);
}
else
remove (ctx->path);
+ unlock_realpath (ctx);
+
return 0;
}
/**
* check_mailbox - Check for changes in the compressed file
* @ctx: Mailbox
*
* If the compressed file changes in size but the mailbox hasn't been changed
@@ -824,20 +839,30 @@
return -1;
if (!ci->close)
{
mutt_error (_("Can't sync a compressed file without a close-hook"));
return -1;
}
- int rc = execute_command (ctx, ci->close, 1, _("Compressing %s"));
+ /* TODO: need to refactor sync so we can lock around the
+ * path sync as well as the compress operation */
+ if (!lock_realpath (ctx, 1))
+ {
+ mutt_error (_("Unable to lock mailbox!"));
+ return -1;
+ }
+
+ int rc = execute_command (ctx, ci->close, _("Compressing %s"));
if (rc == 0)
return -1;
+ unlock_realpath (ctx);
+
store_size (ctx);
return 0;
}
/**
* mutt_comp_valid_command - Is this command string allowed?
* @cmd: Command string
