Re: serious bug. Evolution and Microsoft mentality.
Actually the attached patch is the correct one. There is no need to
memset and you should use PATH_MAX rather than 4096.
Jeff
On Wed, 2002-01-09 at 23:36, Jonathan Walther wrote:
On Wed, Jan 09, 2002 at 07:38:43PM -0500, Jeffrey Stedfast wrote:
Oops, copy/paste-o when migrating the patch to the 1-0 code base.
Here is the correct patch for the 1.0.x branch. Hopefully the Debian
maintainer will apply it? I am creating an Evolution 1.0-5.1 package
on my system with the patch applied. I haven't seen so many signal 11's
in aeons.
Index: camel/providers/local/camel-local-folder.c
===
RCS file: /cvs/gnome/evolution/camel/providers/local/camel-local-folder.c,v
retrieving revision 1.22
diff -u -r1.22 camel-local-folder.c
--- evolution/camel/providers/local/camel-local-folder.c2001/10/28
05:10:55 1.22
+++ evolution/camel/providers/local/camel-local-folder.c2002/01/09
21:44:15
@@ -23,8 +23,9 @@
#include config.h
#endif
#include stdlib.h
+#include limits.h
#include sys/types.h
#include dirent.h
#include sys/stat.h
#include unistd.h
@@ -173,8 +174,9 @@
CamelFolder *folder;
const char *root_dir_path, *name;
struct stat st;
int forceindex;
+ char folder_path[4096];
folder = (CamelFolder *)lf;
name = strrchr(full_name, '/');
@@ -190,8 +192,16 @@
lf-base_path = g_strdup(root_dir_path);
lf-folder_path = g_strdup_printf(%s/%s, root_dir_path, full_name);
lf-summary_path = g_strdup_printf(%s/%s.ev-summary, root_dir_path,
full_name);
lf-index_path = g_strdup_printf(%s/%s.ibex, root_dir_path,
full_name);
+
+ /* follow any symlinks to the mailbox */
+ memset(folder_path, 0, sizeof folder_path);
+ if (lstat (lf-folder_path, st) != -1 S_ISLNK (st.st_mode)
+ realpath (lf-folder_path, folder_path) != NULL) {
+ g_free (lf-folder_path);
+ lf-folder_path = g_strdup (folder_path);
+ }
lf-changes = camel_folder_change_info_new();
/* if we have no index file, force it */
--
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
[EMAIL PROTECTED] - www.ximian.com
Index: ChangeLog
===
RCS file: /cvs/gnome/evolution/camel/ChangeLog,v
retrieving revision 1.1311.2.11
diff -u -r1.1311.2.11 ChangeLog
--- ChangeLog 2002/01/04 22:17:52 1.1311.2.11
+++ ChangeLog 2002/01/10 17:30:10
@@ -1,3 +1,11 @@
+2002-01-09 Jeffrey Stedfast [EMAIL PROTECTED]
+
+ * providers/local/camel-local-folder.c
+ (camel_local_folder_construct): If the mbox file is a symlink,
+ follow the symlink and get the One True Path so that we can
+ rewrite the mbox later without worrying about clobbering the
+ symlink.
+
2001-12-12 Jeffrey Stedfast [EMAIL PROTECTED]
* camel-folder-summary.c (content_info_load): Don't try setting a
Index: providers/local/camel-local-folder.c
===
RCS file: /cvs/gnome/evolution/camel/providers/local/camel-local-folder.c,v
retrieving revision 1.22
diff -u -r1.22 camel-local-folder.c
--- providers/local/camel-local-folder.c2001/10/28 05:10:55 1.22
+++ providers/local/camel-local-folder.c2002/01/10 17:30:10
@@ -24,6 +24,7 @@
#endif
#include stdlib.h
+#include limits.h
#include sys/types.h
#include dirent.h
#include sys/stat.h
@@ -172,6 +173,7 @@
CamelFolderInfo *fi;
CamelFolder *folder;
const char *root_dir_path, *name;
+ char folder_path[PATH_MAX];
struct stat st;
int forceindex;
@@ -191,7 +193,14 @@
lf-folder_path = g_strdup_printf(%s/%s, root_dir_path, full_name);
lf-summary_path = g_strdup_printf(%s/%s.ev-summary, root_dir_path,
full_name);
lf-index_path = g_strdup_printf(%s/%s.ibex, root_dir_path,
full_name);
-
+
+ /* follow any symlinks to the mailbox */
+ if (lstat (lf-folder_path, st) != -1 S_ISLNK (st.st_mode)
+ realpath (lf-folder_path, folder_path) != NULL) {
+ g_free (lf-folder_path);
+ lf-folder_path = g_strdup (folder_path);
+ }
+
lf-changes = camel_folder_change_info_new();
/* if we have no index file, force it */
Re: serious bug. Evolution and Microsoft mentality.
While I agree with your statements, it's strictly not possible in this case - realpath() requires that the second argument be a string buffer of size PATH_MAX (read the manpage). SYNOPSIS #include stdlib.h char *realpath(const char *file_name, char *resolved_name); DESCRIPTION The realpath() function derives, from the pathname pointed to by file_name, an absolute pathname that names the same file, whose resolution does not involve ., .., or sym- bolic links. The generated pathname is stored, up to a max- imum of PATH_MAX bytes, in the buffer pointed to by resolved_name. Jeff On Thu, 2002-01-10 at 13:27, Jeroen Dekkers wrote: On Thu, Jan 10, 2002 at 12:38:30PM -0500, Jeffrey Stedfast wrote: Actually the attached patch is the correct one. There is no need to memset and you should use PATH_MAX rather than 4096. Both is incorrect. PATH_MAX isn't required by POSIX and some systems don't have it (the Hurd for example). The Hurd simply doesn't have a limit (the GNU Coding Standards says arbitrary limits should be avoided, they are bad (think about the 640k limit, 1024 cylinder limit, etc)). If we have to define some limit it would be so large that it is not suitable for static allocation. You should use dynamic allocation and not limit you in any way. Jeroen Dekkers -- Jabber supporter - http://www.jabber.org Jabber ID: [EMAIL PROTECTED] Debian GNU supporter - http://www.debian.org http://www.gnu.org IRC: [EMAIL PROTECTED] -- Jeffrey Stedfast Evolution Hacker - Ximian, Inc. [EMAIL PROTECTED] - www.ximian.com
Re: serious bug. Evolution and Microsoft mentality.
On Wed, 2002-01-09 at 01:43, Jonathan Walther wrote: On Mon, Jan 07, 2002 at 01:56:24PM -0500, Jeffrey Stedfast wrote: Yea, this is kinda painful currently but hopefully by 1.2 this will be much easier. We plan on making it so that you can add a new account using Standard Unix Mail Spool as the source type and pointing it at a directory and have our code automatically show all the mailboxes in the directory. Currently it will only accept single mbox files as sources. That will be very nice. Will you also let us point to Maildir's and have it Just Work? Maildir format allows us to continue using procmail and related programs without worrying about locking issues between Evolution and outside mail delivery programs and filters. Actually, Evolution already supports pointing to outside Maildir directories ;-) It's not really a good idea to use symlinks because Evolution will lock the symlink file and if you have procmail running, it will lock the real mbox file and so if you try and access the same mbox file with both Evolution and procmail, things will end badly for you (ie corrupt mailbox). Correct. Thats why I plan to switch over completely to Maildir format. What happened? My symlink was erased! Gone! A new mbox file had been created from the contents of the original. Did you delete/expunge messages? I presume that you must have because otherwise the mbox would not have been rewritten. The only way to remove Unfortunately, no. I made no changes whatsoever to the mailboxes. I just entered them to see if the messages showed up, they did, then I exited. Thats when I noticed the symlinks had been blown away, and the resulting copied mailbox was bigger in size than the original! Ah, I bet I know why... Evolution added an X-Evolution header to each message for status purposes. The X-Evolution header contains an encoded UID and message flags. That would also explain why it got bigger than the original file. messages from an mbox file is to rewrite it to a new tmp file (minus the messages to be deleted) and then rename that tmp file back to its original name. Unfortunately, UNIX's rename() function clobbers symlinks and thus your problem. Well, alright. I did some research tonight. Before calling rename(), a call to realpath() would resolve the symlink to the correct real filename, and using that would give the correct result. An extra 3 lines of code (to find out how much storage is needed for the real path, to allocate it, then finally to call realpath()). Actually, this isn't guaranteed to work :-( rename() is unfortunately limited to renaming files on the same file system, once you add symlinks to the equation - you could be leaping across file system boundaries and there isn't a way to tell (well, ok, so you can check st.st_dev and compare the 2). The only way I can think of to get around this is to create the tmp mbox file in the same directory as the original (after being realpath()'d). This may also have problems - what if you don't have write-permission in that directory? Actually, Evolution plays very nice with other mail software on the system - you just have to not use symlinks. Supporting symlinks is easy. Not supporting them is un-Unixy. Not as easy as you seem to think, you missed that rename doesn't work across file systems ;-) Anyways, I'll look into it... Evolution locks all mailboxes when it goes to access them to avoid mailbox corruption and so as long as the other system mail software does the same (which it should) then they will all live in harmony. Again, provided one uses Maildir mailboxes, things will be fine. But the thought occurs, Evolution should do its locking on the file returned from realpath() too. You are probably right. -- Jeffrey Stedfast Evolution Hacker - Ximian, Inc. [EMAIL PROTECTED] - www.ximian.com
Re: serious bug. Evolution and Microsoft mentality.
On Wed, 2002-01-09 at 16:20, Jonathan Walther wrote:
On Wed, Jan 09, 2002 at 01:41:28PM -0500, Jeffrey Stedfast wrote:
Unfortunately, no. I made no changes whatsoever to the mailboxes. I
just entered them to see if the messages showed up, they did, then I
exited. Thats when I noticed the symlinks had been blown away, and the
resulting copied mailbox was bigger in size than the original!
Ah, I bet I know why... Evolution added an X-Evolution header to each
message for status purposes. The X-Evolution header contains an encoded
UID and message flags. That would also explain why it got bigger than
the original file.
Fair enough. Pine does something similar. Can it be gauranteed that
the Quoted Printable encoding hasn't been decoded then reencoded in
the process?
not easily. Michael Zucchi and I will have to redesign libcamel to be
able to guarantee this and so must wait until we have time.
Actually, this isn't guaranteed to work :-(
rename() is unfortunately limited to renaming files on the same file
system, once you add symlinks to the equation - you could be leaping
across file system boundaries and there isn't a way to tell (well, ok,
so you can check st.st_dev and compare the 2).
The only way I can think of to get around this is to create the tmp mbox
file in the same directory as the original (after being realpath()'d).
This may also have problems - what if you don't have write-permission in
that directory?
You solved the problem. That is the correct solution. After running
realpath(), use dirname() and make the tmpfile in the same directory
as the mailbox.
[snip]
The attached patch will fix this issue.
Again, provided one uses Maildir mailboxes, things will be fine. But
the thought occurs, Evolution should do its locking on the file
returned from realpath() too.
You are probably right.
I've thought about it some more, and I'm upgrading my maybe to a
strong this is the proper way to do it. Symlinks should not be
locked. They should be followed with realpath() and the real mailbox
should be locked, like other MUA's do. This will truly make it
compatible and play nicely on the Unix system.
It also fixes this.
--
Jeffrey Stedfast
Evolution Hacker - Ximian, Inc.
[EMAIL PROTECTED] - www.ximian.com
Index: ChangeLog
===
RCS file: /cvs/gnome/evolution/camel/ChangeLog,v
retrieving revision 1.1311.2.11
diff -u -r1.1311.2.11 ChangeLog
--- ChangeLog 2002/01/04 22:17:52 1.1311.2.11
+++ ChangeLog 2002/01/09 21:44:15
@@ -1,3 +1,11 @@
+2002-01-09 Jeffrey Stedfast [EMAIL PROTECTED]
+
+ * providers/local/camel-local-folder.c
+ (camel_local_folder_construct): If the mbox file is a symlink,
+ follow the symlink and get the One True Path so that we can
+ rewrite the mbox later without worrying about clobbering the
+ symlink.
+
2001-12-12 Jeffrey Stedfast [EMAIL PROTECTED]
* camel-folder-summary.c (content_info_load): Don't try setting a
Index: providers/local/camel-local-folder.c
===
RCS file: /cvs/gnome/evolution/camel/providers/local/camel-local-folder.c,v
retrieving revision 1.22
diff -u -r1.22 camel-local-folder.c
--- providers/local/camel-local-folder.c2001/10/28 05:10:55 1.22
+++ providers/local/camel-local-folder.c2002/01/09 21:44:15
@@ -24,6 +24,7 @@
#endif
#include stdlib.h
+#include limits.h
#include sys/types.h
#include dirent.h
#include sys/stat.h
@@ -191,7 +192,14 @@
lf-folder_path = g_strdup_printf(%s/%s, root_dir_path, full_name);
lf-summary_path = g_strdup_printf(%s/%s.ev-summary, root_dir_path,
full_name);
lf-index_path = g_strdup_printf(%s/%s.ibex, root_dir_path,
full_name);
-
+
+ /* follow any symlinks to the mailbox */
+ if (lstat (lf-folder_path, st) != -1 S_ISLNK (st.st_mode)
+ realpath (lf-folder_path, folder_path) != NULL) {
+ g_free (lf-folder_path);
+ lf-folder_path = g_strdup (folder_path);
+ }
+
lf-changes = camel_folder_change_info_new();
/* if we have no index file, force it */
Re: serious bug. Evolution and Microsoft mentality.
Oops, copy/paste-o when migrating the patch to the 1-0 code base. Jeff On Wed, 2002-01-09 at 19:28, Jonathan Walther wrote: Thank you for the patch. To make it work, you need to define the variable folder_path. I would recommend this: char folder_path[4096]; And then before using it, do this: memset(folder_path, 0, sizeof folder_path); Cheers. Jonathan On Wed, Jan 09, 2002 at 04:57:34PM -0500, Jeffrey Stedfast wrote: You solved the problem. That is the correct solution. After running realpath(), use dirname() and make the tmpfile in the same directory as the mailbox. [snip] The attached patch will fix this issue. Again, provided one uses Maildir mailboxes, things will be fine. But the thought occurs, Evolution should do its locking on the file returned from realpath() too. You are probably right. I've thought about it some more, and I'm upgrading my maybe to a strong this is the proper way to do it. Symlinks should not be locked. They should be followed with realpath() and the real mailbox should be locked, like other MUA's do. This will truly make it compatible and play nicely on the Unix system. It also fixes this. -- Jeffrey Stedfast Evolution Hacker - Ximian, Inc. [EMAIL PROTECTED] - www.ximian.com
