Re: [PATCH] Re: mcedit stackdumps on exit

[Pavel Tsekov <ptsekov gmx net>]

Hello,

On Wed, 22 Jun 2005, Leonard den Ottolander wrote:

> :) ). I haven't quite grasped the code path. If I understand your
> explanation correctly the first free takes place in clean_dir() and the
> second in mc_maybe_editor_or_viewer(). But after the free in clean_dir()
> fname gets assigned 0 so I don't quite see how the g_free(path) could
> cause a double free condition. But as I said, I don't fully grasp the
> code path yet.

The `fname' pointer in clean_dir() points to a block of memory . The
`path' pointer in mc_maybe_editor_or_viewer() points to the same block of
memory. `fname' and `path' live at different locations in MC's address
space, so zeroing one of them doesn't zero the other one.

> > 3) expand_format() is changed so that it will use the `filename' member of
> >    WEdit if the MC is started as `mcedit'. `mc_get_current_wd' will be
> >    used to determine the current directory `mcedit' mode instead of
> >    `panel->cwd'. Finally the code just eats  the `u' and `t' format
> >    specifiers when in `mcedit' mode.
>
> Just a few remarks/questions:
>
> - In the second hunk for src/user.c you test for panel, but that test
> seems redundant as in the previous else for edit_one_file != NULL you
> assign panel. Shouldn't you skip the test for panel and get the
> assignment of fname inside brackets?

No. The condition `edit_one_file != NULL' is valid only when you invoke
the editor as mcedit (directly from the command line) and not from within
the file manager i.e. pressing F4 on a file. I use `edit_one_file' to
recognize the difference. If the editor is called from the file
manager it is ok to use `panel' because it contains valid data.

Good question though - I should have made it clear when I've posted the
patch.

> - Is the test in the fifth hunk of src/user.c correct? Not totally
> grasping that code, but as you test for (!panel) in the 6th hunk (fall
> through) I was wondering if the test there shouldn't read (panel &&
> !panel->marked).

This question should be answered by the explanation above and the
clarification below.

@@ -235,7 +253,7 @@ expand_format (struct WEdit *edit_widget
 	    return (*quote_func) (menu, 0);
 	break;
     case 's':
-	if (!panel->marked)
+	if (!panel || !panel->marked)
 	    return (*quote_func) (fname, 0);

The meaning of the old code is - if there aren't any selected items in the
panel return the name of the file under the cursor. I have extended it to
do the same thing if expand_format() is invoked from mcedit (!panel). It
is obvious that when you are not using the file manager there is no way to
select files so we just return the name of the file being edited.