Re: mcedit bugs?

[Pavel Roskin <proski gnu org>]

Hello!

> Until recently I've been using RedHat 6.2 and mc shipped with it
> (4.5.42-10). Now when I've switched to a newer version I've found out that
> mcedit's behaviour changed somewhat. I am wondering if these changes are
> deliberate or just bugs introduced?

It's a bug.

The code is apparently written in a hurry, without testing.  
check_file_access() is not documented, but I guess the intention was that
it should return 2 in the file is created.

>         if (r == 2)
>             return edit->delete_file = 1;

It's very bad code.  This statement uses 1 both as the value of
edit->delete_file and the return value.  What if somebody decides to
invert the return value of this function?  It's very easy to break this
code by simple changes.

I'm applying this patch:

==================================
--- ChangeLog
+++ ChangeLog
@@ -1 +1,10 @@
+2002-06-24  Pavel Roskin  <proski gnu org>
+
+	* edit.c (check_file_access): Return 1 on all errors, document
+	this behavior.  Set edit->delete_file to 1 for newly created
+	files.
+	(edit_open_file): Don't set edit->delete_file, it's now done in
+	check_file_access().
+	Reported by Saso <saso bojler dhs org>
+
 2002-05-13  Andrew V. Samoilov  <kai cmail ru>
--- edit.c
+++ edit.c
@@ -398,6 +398,7 @@ int edit_insert_file (WEdit * edit, cons
     return 1;
 }
 
+/* Open file and create it if necessary.  Return 0 for success, 1 for error.  */
 static int check_file_access (WEdit *edit, const char *filename, struct stat *st)
 {
     int file;
@@ -417,7 +418,7 @@ static int check_file_access (WEdit *edi
     /* Open the file, create it if needed */
     if ((file = mc_open (filename, O_RDONLY | O_CREAT, 0666)) < 0) {
 	edit_error_dialog (_ (" Error "), get_sys_error (catstrs (_ (" Failed trying to open file for reading: "), filename, " ", 0)));
-	return 2;
+	return 1;
     }
 
     /* If the file has just been created, we don't have valid stat yet, so do it now */
@@ -428,6 +429,11 @@ static int check_file_access (WEdit *edi
     }
     mc_close (file);
 
+    /* If it's a new file, delete it if it's not modified or saved */
+    if (!stat_ok && st->st_size == 0) {
+	edit->delete_file = 1;
+    }
+
     if (st->st_size >= SIZE_LIMIT) {
 /* The file-name is printed after the ':' */
 	edit_error_dialog (_ (" Error "), catstrs (_ (" File is too large: "), \
@@ -447,8 +453,6 @@ int edit_open_file (WEdit * edit, const 
     } else {
 	int r;
 	r = check_file_access (edit, filename, &st);
-	if (r == 2)
-	    return edit->delete_file = 1;
 	if (r)
 	    return 1;
 	edit->stat1 = st;
==================================

-- 
Regards,
Pavel Roskin