Hi, and sorry for the late followup.
Imho there are 3 issues left in the umask handling:
#1: main.c sets umask(077) unconditionally. Should be removed.
#2: Even after fixing #1, the original process umask is not respected
when creating files. One could argue that there should be a way to
tell mutt "don't mess with my umask", e.g. by setting umask=0.
#3: The current implementation sets/resets the umask all the time. The
advantage is that subshells created should still have the parent umask
(after item #1 is fixed), but the places where umask calls are done
are scattered all over the code and look overly complex.
Below is an alternative patch (on top of the current one) that fixes
(changes) all 3 items. Arguably, subshells will now have a modified
umask, but that could be declared a feature (or not?).
Depending on the discussion I can prepare a new patch with a different
solution to #2 (which bugs me most personally) and/or #3 (the current
behavior is fine, just not what I would have expected, see also Debian
bug #179119).
Comments?
diff -r c2c37013cdd9 attach.c
--- a/attach.c Fri Mar 16 01:15:47 2007 +0100
+++ b/attach.c Fri Mar 16 12:48:43 2007 +0100
@@ -736,21 +736,12 @@ static FILE *
static FILE *
mutt_save_attachment_open (char *path, int flags)
{
- mode_t omask;
- FILE *fp = NULL;
-
- omask = umask(Umask);
-
if (flags == M_SAVE_APPEND)
- fp = fopen (path, "a");
- else if (flags == M_SAVE_OVERWRITE)
- fp = fopen (path, "w"); /* __FOPEN_CHECKED__ */
- else
- fp = safe_fopen (path, "w");
-
- umask(omask);
-
- return fp;
+ return fopen (path, "a");
+ if (flags == M_SAVE_OVERWRITE)
+ return fopen (path, "w"); /* __FOPEN_CHECKED__ */
+
+ return safe_fopen (path, "w");
}
/* returns 0 on success, -1 on error */
diff -r c2c37013cdd9 globals.h
--- a/globals.h Fri Mar 16 01:15:47 2007 +0100
+++ b/globals.h Fri Mar 16 12:51:18 2007 +0100
@@ -201,6 +201,7 @@ WHERE short SleepTime INITVAL (1);
WHERE short SleepTime INITVAL (1);
WHERE short Timeout;
WHERE short Umask;
+WHERE short Umask_orig;
WHERE short Wrap;
WHERE short WriteInc;
diff -r c2c37013cdd9 init.c
--- a/init.c Fri Mar 16 01:15:47 2007 +0100
+++ b/init.c Fri Mar 16 12:52:52 2007 +0100
@@ -1564,6 +1564,9 @@ static void mutt_restore_default (struct
set_quadoption (p->data, p->init);
break;
case DT_NUM:
+ if (mutt_strcmp (p->option, "umask") == 0)
+ umask(Umask_orig | p->init);
+ /* fallthrough */
case DT_SORT:
case DT_MAGIC:
*((short *) p->data) = p->init;
@@ -2014,6 +2017,8 @@ static int parse_set (BUFFER *tmp, BUFFE
if (*ptr < 0)
*ptr = 0;
}
+ else if (mutt_strcmp (MuttVars[idx].option, "umask") == 0)
+ umask(Umask_orig | *ptr);
else if (mutt_strcmp (MuttVars[idx].option, "wrapmargin") == 0)
{
if (*ptr < 0)
diff -r c2c37013cdd9 init.h
--- a/init.h Fri Mar 16 01:15:47 2007 +0100
+++ b/init.h Fri Mar 16 12:42:33 2007 +0100
@@ -2913,6 +2913,7 @@ struct option_t MuttVars[] = {
/*
** .pp
** Sets the umask to use when creating mailboxes or saving attachments.
+ ** The current process umask is ORed with this value.
*/
{ "use_8bitmime", DT_BOOL, R_NONE, OPTUSE8BITMIME, 0 },
diff -r c2c37013cdd9 main.c
--- a/main.c Fri Mar 16 01:15:47 2007 +0100
+++ b/main.c Fri Mar 16 12:51:18 2007 +0100
@@ -568,7 +568,7 @@ int main (int argc, char **argv)
mutt_error = mutt_nocurses_error;
mutt_message = mutt_nocurses_error;
SRAND (time (NULL));
- umask (077);
+ umask (Umask_orig = umask (0));
memset (Options, 0, sizeof (Options));
memset (QuadOptions, 0, sizeof (QuadOptions));
diff -r c2c37013cdd9 mx.c
--- a/mx.c Fri Mar 16 01:15:47 2007 +0100
+++ b/mx.c Fri Mar 16 12:50:04 2007 +0100
@@ -492,7 +492,6 @@ static int mx_open_mailbox_append (CONTE
static int mx_open_mailbox_append (CONTEXT *ctx, int flags)
{
struct stat sb;
- mode_t omask;
ctx->append = 1;
@@ -503,8 +502,6 @@ static int mx_open_mailbox_append (CONTE
#endif
- omask = umask(Umask);
-
if(stat(ctx->path, &sb) == 0)
{
ctx->magic = mx_get_magic (ctx->path);
@@ -529,7 +526,7 @@ static int mx_open_mailbox_append (CONTE
if (mkdir (ctx->path, S_IRWXU|S_IRWXG|S_IRWXO))
{
mutt_perror (ctx->path);
- goto err_umask;
+ return (-1);
}
if (ctx->magic == M_MAILDIR)
@@ -539,7 +536,7 @@ static int mx_open_mailbox_append (CONTE
{
mutt_perror (tmp);
rmdir (ctx->path);
- goto err_umask;
+ return (-1);
}
snprintf (tmp, sizeof (tmp), "%s/new", ctx->path);
@@ -549,7 +546,7 @@ static int mx_open_mailbox_append (CONTE
snprintf (tmp, sizeof (tmp), "%s/cur", ctx->path);
rmdir (tmp);
rmdir (ctx->path);
- goto err_umask;
+ return (-1);
}
snprintf (tmp, sizeof (tmp), "%s/tmp", ctx->path);
if (mkdir (tmp, S_IRWXU|S_IRWXG|S_IRWXO))
@@ -560,7 +557,7 @@ static int mx_open_mailbox_append (CONTE
snprintf (tmp, sizeof (tmp), "%s/new", ctx->path);
rmdir (tmp);
rmdir (ctx->path);
- goto err_umask;
+ return (-1);
}
}
else
@@ -572,7 +569,7 @@ static int mx_open_mailbox_append (CONTE
{
mutt_perror (tmp);
rmdir (ctx->path);
- goto err_umask;
+ return (-1);
}
close (i);
}
@@ -581,7 +578,7 @@ static int mx_open_mailbox_append (CONTE
else
{
mutt_perror (ctx->path);
- goto err_umask;
+ return (-1);
}
switch (ctx->magic)
@@ -598,7 +595,7 @@ static int mx_open_mailbox_append (CONTE
mutt_error (_("Couldn't lock %s\n"), ctx->path);
safe_fclose (&ctx->fp);
}
- goto err_umask;
+ return (-1);
}
fseek (ctx->fp, 0, 2);
break;
@@ -609,15 +606,10 @@ static int mx_open_mailbox_append (CONTE
break;
default:
- goto err_umask;
- }
-
- umask(omask);
+ return (-1);
+ }
+
return 0;
-
- err_umask:
- umask(omask);
- return -1;
}
/*
@@ -1266,7 +1258,6 @@ MESSAGE *mx_open_new_message (CONTEXT *d
MESSAGE *msg;
int (*func) (MESSAGE *, CONTEXT *, HEADER *);
ADDRESS *p = NULL;
- mode_t omask;
switch (dest->magic)
{
@@ -1305,8 +1296,6 @@ MESSAGE *mx_open_new_message (CONTEXT *d
if(msg->received == 0)
time(&msg->received);
-
- omask = umask(Umask);
if (func (msg, dest, hdr) == 0)
{
@@ -1331,8 +1320,6 @@ MESSAGE *mx_open_new_message (CONTEXT *d
}
else
FREE (&msg);
-
- umask(omask);
return msg;
}
Christoph
--
cb@xxxxxxxx | http://www.df7cb.de/
Attachment:
signature.asc
Description: Digital signature