Re: [PATCHES] awaiting...
On Sun, May 23, 2004 at 08:27:22PM +0900, TAKAHASHI Tamotsu wrote:
> I recently posted multiple patches.
> Please commit these changes to cvs if you don't find a bug in them.
> These really fix problems, and I have not experienced any side effect.
> I have been testing them for at least three days.
My unbind patch has been discussed and refused.
(IIRC, Dave will write better one.)
But nobody has objected to the rest two patches.
I try to explain these patches again.
> Date: Fri, 7 May 2004 16:23:57 +0900
> Subject: Re: Using the builtin editor
>
> --- compose.c~ Tue Apr 13 09:01:33 2004
> +++ compose.c Fri May 7 16:10:37 2004
> @@ -733,7 +733,7 @@
> /* fall through */
> case OP_COMPOSE_EDIT_HEADERS:
> if (op == OP_COMPOSE_EDIT_HEADERS ||
> - (op == OP_COMPOSE_EDIT_MESSAGE && option (OPTEDITHDRS)))
> + (op == OP_COMPOSE_EDIT_MESSAGE && option (OPTEDITHDRS) &&
> (mutt_strcmp ("builtin", Editor) != 0)))
> {
> char *tag = NULL, *err = NULL;
> mutt_env_to_local (msg->env);
You will be here if you do "edit-message" on compose menu.
: case OP_COMPOSE_EDIT_MESSAGE:
: if (Editor && (mutt_strcmp ("builtin", Editor) != 0) && !option
(OPTEDITHDRS))
: {
: mutt_edit_file (Editor, msg->content->filename);
: mutt_update_encoding (msg->content);
: menu->redraw = REDRAW_CURRENT | REDRAW_STATUS;
: break;
: }
: /* fall through */
And fall through when your $editor=="builtin".
: case OP_COMPOSE_EDIT_HEADERS:
: if (op == OP_COMPOSE_EDIT_HEADERS ||
: (op == OP_COMPOSE_EDIT_MESSAGE && option (OPTEDITHDRS)))
: {
: char *tag = NULL, *err = NULL;
: mutt_env_to_local (msg->env);
: mutt_edit_headers (NONULL (Editor), msg->content->filename, msg,
: fcc, fcclen);
Then mutt_edit_headers() calls "builtin" as an external editor. (BUG!)
: if (mutt_env_to_idna (msg->env, &tag, &err))
: {
: mutt_error (_("Bad IDN in \"%s\": '%s'"), tag, err);
: FREE (&err);
: }
: }
: else
: {
: /* this is grouped with OP_COMPOSE_EDIT_HEADERS because the
: attachment list could change if the user invokes ~v to edit
: the message with headers, in which we need to execute the
: code below to regenerate the index array */
: mutt_builtin_editor (msg->content->filename, msg, cur);
: }
Reading this comment, you must think we have to call
mutt_builtin_editor() instead of mutt_edit_headers().
> Date: Sat, 15 May 2004 14:34:40 +0900
> Subject: Re: [BETTER PATCH] Re: Bug#237426: Mutt segfaults on unreadable
> directories
>
> --- browser.c.old Wed Nov 5 18:41:31 2003
> +++ browser.c Sat May 15 14:29:52 2004
> @@ -944,21 +944,21 @@
> {
> if (S_ISDIR (st.st_mode))
> {
> - strfcpy (LastDir, buf, sizeof (LastDir));
> destroy_state (&state);
> - if (examine_directory (menu, &state, LastDir, prefix) == 0)
> - {
> - menu->current = 0;
> - menu->top = 0;
> - init_menu (&state, menu, title, sizeof (title), buffy);
> - }
> + if (examine_directory (menu, &state, buf, prefix) == 0)
> + strfcpy (LastDir, buf, sizeof (LastDir));
> else
> {
> mutt_error _("Error scanning directory.");
> - destroy_state (&state);
> - mutt_menuDestroy (&menu);
> - goto bail;
> + if (examine_directory (menu, &state, LastDir, prefix) == -1)
> + {
> + mutt_menuDestroy (&menu);
> + goto bail;
> + }
> }
> + menu->current = 0;
> + menu->top = 0;
> + init_menu (&state, menu, title, sizeof (title), buffy);
> }
> else
> mutt_error (_("%s is not a directory."), buf);
Original browser.c has a couple of problems:
: if (S_ISDIR (st.st_mode))
: {
: strfcpy (LastDir, buf, sizeof (LastDir));
LastDir changes, even if buf fails to be examined. So we can't get back to
LastDir.
: destroy_state (&state);
First destruction.
: if (examine_directory (menu, &state, LastDir, prefix) == 0)
: {
: menu->current = 0;
: menu->top = 0;
: init_menu (&state, menu, title, sizeof (title), buffy);
: }
: else
: {
: mutt_error _("Error scanning directory.");
: destroy_state (&state);
Second destruction. (BUG!)
: mutt_menuDestroy (&menu);
: goto bail;
: }
: }
Users must be surprised if mutt goes to bail (LastDirBackup) suddenly.
Because users normally expect to be back to LastDir when an error occurs.
After applying my patch:
: if (S_ISDIR (st.st_mode))
: {
: destroy_state (&state);
: if (examine_directory (menu, &state, buf, prefix) == 0)
: strfcpy (LastDir, buf, sizeof (LastDir));
Only successfully-examined directories should be strfcpy'ed to LastDir.
: else
: {
: mutt_error _("Error scanning directory.");
: if (examine_directory (menu, &state, LastDir, prefix) == -1)
: {
: mutt_menuDestroy (&menu);
: goto bail;
: }
When buf failed, Mutt should go back to LastDir.
If LastDir also failed, goto bail (to go to LastDirBackup).
: }
: menu->current = 0;
: menu->top = 0;
: init_menu (&state, menu, title, sizeof (title), buffy);
: }
Any reason not to check-in these patches?
--
tamo