Re: [evolution-patches] patch for bug 48759

[Charles Zhang <Charles Zhang Sun Com>]

Jeffrey Stedfast wrote:

On Wed, 2003-09-24 at 11:37, Charles Zhang wrote:
  

So I prefer this one:
        char *pathname = g_path_get_dirname (filename);          //
get it's pathdir
        if (stat (pathname, &st) == -1)                               
// if the pathdir doestn't exist, then ok doesn't work
                return TRUE;
        else if (! S_ISDIR (st.st_mode))                             
// if the pathdir isn't a dir, then ok doesn't work
                return TRUE;
    

those 2 if's are not neccessary. if we can't stat the filename, it
doesn't matter if the parent directory exists or not.
  

        else if (stat (filename, &st) == -1)                          
// if the file doesn't exist, then ok works
                return FALSE;
        else if (! S_ISREG (st.st_mode))                             
// if the file isn't a regular file, then ok doesn't work
                return TRUE;
        else
                return FALSE;
                // else, ok works
    

what you want is this:

	if (stat (filename, &st) == 0 && !S_ISREG (st.st_mode))
		return TRUE;
	
	return FALSE;
  

Why I write so many creteria is to handle the case of "incorrect
filename" and "file doesn't exist" separately.
For example, I can leave the "~/does_not_exist" filename to the upper
caller to process,
but I cannot leave the "~/does_not_exist/a.msg" filename to the upper
caller to process.
    

it is by far simpler to just do what my code does above, it is by far
simpler.

What I insist is that: if the user specify a filename, which doesn't exist and whose folder doesn't exist either, we should stop the following process.
You mean that we can leave that filename to the upper caller to handle and it will create the necessary folders recursively? But it doesn't now.
If we just return a filename with a wrong folder name to the upper, the same bug appears now.
What should we do? Stop this filename whose pathdir doesn't exist, or raise another bug of no parsing path-file name?

I have altered my patch.

Such we should change the origin instrument
from
"  run_selector(composer, _("Attach file(s)"), TRUE, NULL);  "
to
"  run_selector(composer, title, FALSE, NULL);  "
    

yes, this is correct I think.

just change the stat calls to what I suggested above.

Jeff
  

Then should I modify this instrument or just write some comment and leave it to others?

Thanks you, Jeffrey.

Best Regards
Charles Zhang

Index: composer/e-msg-composer-select-file.c
===================================================================
RCS file: /cvs/gnome/evolution/composer/e-msg-composer-select-file.c,v
retrieving revision 1.25
diff -u -p -r1.25 e-msg-composer-select-file.c
--- composer/e-msg-composer-select-file.c	2 Apr 2003 18:51:01 -0000	1.25
+++ composer/e-msg-composer-select-file.c	25 Sep 2003 06:55:07 -0000
@@ -38,6 +38,29 @@
 
 #include "e-msg-composer-select-file.h"
 
+static gboolean
+ok_press_callback (GtkWidget        *okbutton, 
+		   GdkEventButton   *event,
+		   GtkFileSelection *selection)
+{
+	char *filename = gtk_file_selection_get_filename (selection);
+	char *pathname = g_path_get_dirname (filename);
+	gboolean ret = FALSE;
+	struct stat st;
+
+	if (stat (pathname, &st) == -1 || (! S_ISDIR (st.st_mode))) {
+		/* pathdir doesn't exist or isn't a valid dir */
+		ret = TRUE;
+	}
+	else if (stat (filename, &st) == 0 && (! S_ISREG (st.st_mode))) {
+		/* file exists but isn't a regular file */
+		ret = TRUE;
+	}
+	
+	g_free (pathname);
+	return ret;
+}
+
 static GtkFileSelection *
 run_selector(EMsgComposer *composer, const char *title, int multi, gboolean *showinline_p)
 {
@@ -52,6 +75,10 @@ run_selector(EMsgComposer *composer, con
 	gnome_window_icon_set_from_file((GtkWindow *)selection, EVOLUTION_DATADIR "/images/evolution/compose-message.png");
 	gtk_file_selection_set_select_multiple((GtkFileSelection *)selection, multi);
 
+	g_signal_connect (((GtkFileSelection *) selection)->ok_button,
+			  "button_press_event", G_CALLBACK (ok_press_callback),
+			  (gpointer) selection);
+
 	/* restore last path used */
 	path = g_object_get_data((GObject *)composer, "attach_path");
 	if (path == NULL) {
@@ -98,6 +125,9 @@ e_msg_composer_select_file (EMsgComposer
 {
 	GtkFileSelection *selection;
 	char *name = NULL;
+
+	/* FIXME: the next instrument should be "run_selector(composer, title, FALSE, NULL)",
+	   otherwise this dialog's title is wrong and more than one file can be selected. */
 
 	selection = run_selector(composer, _("Attach file(s)"), TRUE, NULL);
 	if (selection) {