bug#6414: f->output_data.w32->menubar_widget uninitialized?

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

I noticed some system call errors in free_frame_menu_bar so I tried to
nail it down:

void
free_frame_menubar (f)
     FRAME_PTR f;
{
  BLOCK_INPUT;

  {
    HMENU old = GetMenu (FRAME_W32_WINDOW (f));
    DebPrint (("free_frame_menubar.GetMenu.old=%d, f=%d, menubar_widget=%d",
               old, (FRAME_W32_WINDOW (f)),
f->output_data.w32->menubar_widget ));
    if (old)
      {
        if (!SetMenu (FRAME_W32_WINDOW (f), NULL))
          W32ASSERT (0, "free_frame_menubar.SetMenu");
        f->output_data.w32->menubar_widget = NULL;
        if (!DestroyMenu (old))
          W32ASSERT (0, "free_frame_menubar.DestroyMenu");
      }
  }

  UNBLOCK_INPUT;
}

This gave me the output below which seems to indicate that
menubar_widget is uninitialized:

  warning: free_frame_menubar.GetMenu.old=0, f=3146106, menubar_widget=329385

Where should it be initialized?

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Sun, Jun 13, 2010 at 8:01 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
>
> This gave me the output below which seems to indicate that
> menubar_widget is uninitialized:
>
>  warning: free_frame_menubar.GetMenu.old=0, f=3146106, menubar_widget=329385
>
> Where should it be initialized?


Looks like it is initialized at line 4928 in Fx_create_frame in
w32fns.c so the problem must come in later.

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Sun, Jun 13, 2010 at 8:32 PM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Sun, Jun 13, 2010 at 8:01 PM, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>>
>> This gave me the output below which seems to indicate that
>> menubar_widget is uninitialized:
>>
>>  warning: free_frame_menubar.GetMenu.old=0, f=3146106, menubar_widget=329385
>>
>> Where should it be initialized?
>
>
> Looks like it is initialized at line 4928 in Fx_create_frame in
> w32fns.c so the problem must come in later.

The problem seems to be in x_free_frame_resources. Should not
free_frame_menubar be called before my_destroy_window there?

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Sun, Jun 13, 2010 at 20:47, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> The problem seems to be in x_free_frame_resources. Should not
> free_frame_menubar be called before my_destroy_window there?

You reported seeing some system call errors, presumably under a
debugger or with DebPrint statements. Did you see any bug? If so, can
you please send a recipe to reproduce it starting with emacs -Q?

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Sun, Jul 3, 2011 at 20:54, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Sun, Jun 13, 2010 at 20:47, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> The problem seems to be in x_free_frame_resources. Should not
>> free_frame_menubar be called before my_destroy_window there?
>
> You reported seeing some system call errors, presumably under a
> debugger or with DebPrint statements. Did you see any bug? If so, can
> you please send a recipe to reproduce it starting with emacs -Q?

I can't do much to check it now. I do not have time to setup the
required environment on my new pc now.

However I think it is also very difficult to catch such errors. They
are likely to be race conditions (since w32 messages from different
sources are involved).

So I think it is better to check the logic. (As I have tried to above.)

And I also still think it is a good idea to add check for error
conditions after all system calls. (For the same reason as above.)

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Mon, Jul 4, 2011 at 00:19, Lennart Borgman <lennart.borgman@gmail.com> wrote:

> However I think it is also very difficult to catch such errors. They
> are likely to be race conditions (since w32 messages from different
> sources are involved).

But you're talking of presumed errors, i.e., you read the code, see
some error messages from system calls, but there's no error, is there?

> So I think it is better to check the logic. (As I have tried to above.)

Which is fine, but it is not IMO a bug report. In this specific case,
the title refers to an uninitialized struct component, but you
yourself say latter that it is initialized.

> And I also still think it is a good idea to add check for error
> conditions after all system calls. (For the same reason as above.)

As you know, this has been discussed and there's some difference of
opinion, but again, even if you're right, that's not a bug report,
other than perhaps a wishlist, but frankly, this seems more like
something to discuss (again) on emacs-devel than to file as a bug.

So, to summarize: is this bug report about a bug? Is there something
to do about it? Or can we close it?

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Mon, Jul 4, 2011 at 00:30, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, Jul 4, 2011 at 00:19, Lennart Borgman <lennart.borgman@gmail.com> wrote:
>
>> However I think it is also very difficult to catch such errors. They
>> are likely to be race conditions (since w32 messages from different
>> sources are involved).
>
> But you're talking of presumed errors, i.e., you read the code, see
> some error messages from system calls, but there's no error, is there?

It has been some time since I suggested this, but as far as I remember
I looked into this because I had quite a few crashes. They seemed to
be related to menus.

>> So I think it is better to check the logic. (As I have tried to above.)
>
> Which is fine, but it is not IMO a bug report. In this specific case,
> the title refers to an uninitialized struct component, but you
> yourself say latter that it is initialized.

Yes, correct. And I pointed to another place where I thought the
problem was instead.

>> And I also still think it is a good idea to add check for error
>> conditions after all system calls. (For the same reason as above.)
>
> As you know, this has been discussed and there's some difference of
> opinion, but again, even if you're right, that's not a bug report,
> other than perhaps a wishlist, but frankly, this seems more like
> something to discuss (again) on emacs-devel than to file as a bug.

Since you asked for a recipe I thought I had to mention this again. It
looked to me that you overlooked this problem again.

> So, to summarize: is this bug report about a bug? Is there something
> to do about it? Or can we close it?

Did you look at the logic as I suggested? If you are sure my
suggestion is wrong then feel free to close the bug.

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Mon, Jul 4, 2011 at 00:37, Lennart Borgman <lennart.borgman@gmail.com> wrote:

> It has been some time since I suggested this, but as far as I remember
> I looked into this because I had quite a few crashes. They seemed to
> be related to menus.

Yes, I've seen these bug reports, but they usually don't have enough
information to follow through (for what I've seen, Eli tried to get
you to add more information, with mixed results).

> Yes, correct. And I pointed to another place where I thought the
> problem was instead.

Yes, but still not saying what the problem is. You had crashes in
menus, and you filed other bug reports for them. I'm talking about
this "bug" report.

> Since you asked for a recipe I thought I had to mention this again. It
> looked to me that you overlooked this problem again.

"Overlooked [...] again"?

> Did you look at the logic as I suggested?

If the logic is flawed and there's an unitialized struct, it will
likely cause bugs and crashes. We only have a few such reports, by
you, but they are not very informative, often it is not clear whether
it happens with stock Emacs or you patched one (which includes changes
in menu functions), and I don't even know if you've had more such
crashes recently or they were a year or two ago. Lots of things have
changed in between.

> If you are sure my
> suggestion is wrong then feel free to close the bug.

If your suggestion is anything other than "all system calls should be
checked", which has already been discussed in emacs-devel, I fail to
see what it is. Care to clarify?

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Mon, Jul 4, 2011 at 00:48, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, Jul 4, 2011 at 00:37, Lennart Borgman <lennart.borgman@gmail.com> wrote:
>
>> It has been some time since I suggested this, but as far as I remember
>> I looked into this because I had quite a few crashes. They seemed to
>> be related to menus.
>
> Yes, I've seen these bug reports, but they usually don't have enough
> information to follow through (for what I've seen, Eli tried to get
> you to add more information, with mixed results).

I tried to do that, but as I said in another thread I do not see the
same problem in my patched build any more. (But I see new problems on
64-bit windows.)

>> Yes, correct. And I pointed to another place where I thought the
>> problem was instead.
>
> Yes, but still not saying what the problem is. You had crashes in
> menus, and you filed other bug reports for them. I'm talking about
> this "bug" report.

This is a complement to them. We agreed to file bug report for code
changes we should not forget. This is one of those.

Do you agree that this report is a reasonable way to tell you what I
have done to try to fix the problem and get your thoughts on it?
Otherwise suggest something else. (I can't do much now, but perhaps
later.)

>> Since you asked for a recipe I thought I had to mention this again. It
>> looked to me that you overlooked this problem again.
>
> "Overlooked [...] again"?

Yes. The problem with race conditions was one of the main reasons for
my suggestion to add error checking to all system calls. I thought
that you might have overlooked this since you suggested that I should
send a clear recipe for how to reproduce the bug. But maybe you did
not do that? Please explain if I am misunderstanding you.

>> Did you look at the logic as I suggested?
>
> If the logic is flawed and there's an unitialized struct, it will
> likely cause bugs and crashes. We only have a few such reports, by
> you, but they are not very informative, often it is not clear whether
> it happens with stock Emacs or you patched one (which includes changes
> in menu functions), and I don't even know if you've had more such
> crashes recently or they were a year or two ago. Lots of things have
> changed in between.

I think I was wrong about the initialization. As I said later I think
the structure was initialized.

Yes, I have a lot of changes in that area. Complicated, unfortunately.
I think I have lost control of the details there now. And I never felt
I had complete control of it. The code is complex and I am not even
sure that we are always doing things in the right thread etc.

However after fixing the things I suggested I have not seen those
crashes any more as far as I remember. But it might be that I have
changed my usage pattern of Emacs. Nowadays I am mostly using org-mode
with my bibhlp.el extensions.

>> If you are sure my
>> suggestion is wrong then feel free to close the bug.
>
> If your suggestion is anything other than "all system calls should be
> checked", which has already been discussed in emacs-devel, I fail to
> see what it is. Care to clarify?

Yes, it is this that I wrote earlier:

  "The problem seems to be in x_free_frame_resources. Should not
  free_frame_menubar be called before my_destroy_window there?"

We might disagree, but be sure I am glad for your efforts!

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Mon, Jul 4, 2011 at 01:11, Lennart Borgman <lennart.borgman@gmail.com> wrote:

> This is a complement to them. We agreed to file bug report for code
> changes we should not forget. This is one of those.

A bug report called " f->output_data.w32->menubar_widget
uninitialized?" and suggesting to check some system calls return
values as part of some non-very-specific hypothetical bug is not a
"code change we should not forget", IMHO. As I've said in the previous
message, I think you're talking about your suggestion of checking
every system call. If so, please create a wishlist bug report called
"All system calls' return values should be checked", or somesuch, so
at the very least people perusing bug reports can know at a glance
that it is a wishlist and what it is about. I promise I won't close
that bug (though I won't likely do much about it, either).

> Do you agree that this report is a reasonable way to tell you what I
> have done to try to fix the problem and get your thoughts on it?

No, because I still don't know what the problem is.

> Yes. The problem with race conditions was one of the main reasons for
> my suggestion to add error checking to all system calls. I thought
> that you might have overlooked this since you suggested that I should
> send a clear recipe for how to reproduce the bug. But maybe you did
> not do that? Please explain if I am misunderstanding you.

Your "problems with race conditions" do only happen to *you*, on
*your* patched Emacs. So, if you want to help tracking them, you
shouldn't "suggest" anything (IMO, I hasten to add). Patch your Emacs
with debug macros to you heart's content, and when you do find
something wrong, reproduce it with the stock Emacs and file a
(detailed, step-by-step recipe'd) bug report about it.

> Yes, I have a lot of changes in that area. Complicated, unfortunately.
> I think I have lost control of the details there now. And I never felt
> I had complete control of it. The code is complex and I am not even
> sure that we are always doing things in the right thread etc.

"We"? You're talking about code that you've changed in non-trivial ways!

> However after fixing the things I suggested I have not seen those
> crashes any more as far as I remember.

Then, what are we discussing about?

>  "The problem seems to be in x_free_frame_resources. Should not
>  free_frame_menubar be called before my_destroy_window there?"

It is perhaps a fine suggestion, if you care to explain what is "the problem".

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Mon, Jul 4, 2011 at 01:21, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, Jul 4, 2011 at 01:11, Lennart Borgman <lennart.borgman@gmail.com> wrote:
>
>> This is a complement to them. We agreed to file bug report for code
>> changes we should not forget. This is one of those.
>
> A bug report called " f->output_data.w32->menubar_widget
> uninitialized?" and suggesting to check some system calls return
> values as part of some non-very-specific hypothetical bug is not a
> "code change we should not forget", IMHO.

That there is (was) a bug in the menus is not hypothetical. The reason
I am seeing those bugs and not so many other people is probably that I
am using the menus. (Since I do not use Alt as META I can use them
more easily. And I do.)

> As I've said in the previous
> message, I think you're talking about your suggestion of checking
> every system call. If so, please create a wishlist bug report called
> "All system calls' return values should be checked", or somesuch, so
> at the very least people perusing bug reports can know at a glance
> that it is a wishlist and what it is about. I promise I won't close
> that bug (though I won't likely do much about it, either).

OK.

>> Yes. The problem with race conditions was one of the main reasons for
>> my suggestion to add error checking to all system calls. I thought
>> that you might have overlooked this since you suggested that I should
>> send a clear recipe for how to reproduce the bug. But maybe you did
>> not do that? Please explain if I am misunderstanding you.
>
> Your "problems with race conditions" do only happen to *you*, on
> *your* patched Emacs. So, if you want to help tracking them, you
> shouldn't "suggest" anything (IMO, I hasten to add). Patch your Emacs
> with debug macros to you heart's content, and when you do find
> something wrong, reproduce it with the stock Emacs and file a
> (detailed, step-by-step recipe'd) bug report about it.

I think you misunderstand the logic here. The race condition shows up
because of system messages. In your case you probably see much, much
less of those since (I believe) you use menus much less often.

This has nothing to do with my patched build. In fact I started to
patch Emacs because of those problems. And I have had much less
problems in this area with my patched version.

As I have explained quite a few times the refusal to add things like
error checking puts a lot of burden on me. I therefore do not have
time any more to work with Emacs (except for some libraries I use
myself).

>> Yes, I have a lot of changes in that area. Complicated, unfortunately.
>> I think I have lost control of the details there now. And I never felt
>> I had complete control of it. The code is complex and I am not even
>> sure that we are always doing things in the right thread etc.
>
> "We"? You're talking about code that you've changed in non-trivial ways!

No. I am talking about the core Emacs here.

>> However after fixing the things I suggested I have not seen those
>> crashes any more as far as I remember.
>
> Then, what are we discussing about?

You might want to fix the core Emacs.

>>  "The problem seems to be in x_free_frame_resources. Should not
>>  free_frame_menubar be called before my_destroy_window there?"
>
> It is perhaps a fine suggestion, if you care to explain what is "the problem".

Now I am a bit lost. I thought I had done that change, but I have not.
(Or it has been reverted by some merge, that happens.)

I think the suggestion was just because that is what the MS docs suggest.


However I see another small change that I have made to w32term.c that
might have fixed some crashes (but this is unrelated to the problem we
are discussing) :

*** c:/emacs-lp/bld/emacs/trunk/src/w32term.c	2011-03-19
12:38:38.000000000 +0100

--- c:/emacs-lp/bld/emacs/emacsw32/src/w32term.c	2011-07-04
01:37:24.883981800 +0200

***************

*** 4491,4497 ****

  	  else

  	    {

  	      f = x_window_to_frame (dpyinfo, msg.msg.hwnd);

! 	      f->async_visible = msg.msg.wParam;

  	    }

  #endif



--- 4491,4498 ----

  	  else

  	    {

  	      f = x_window_to_frame (dpyinfo, msg.msg.hwnd);

!               if (f)

!                 f->async_visible = msg.msg.wParam;

  	    }

  #endif

 ***************

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Mon, Jul 4, 2011 at 01:42, Lennart Borgman <lennart.borgman@gmail.com> wrote:

> That there is (was) a bug in the menus is not hypothetical.

That you have stumbled upon some bugs in the menus I don't deny. But
this report was not about any bug report, just a system call return
value that did not cause any bug (or at least, such bug was not
reported).

> The reason
> I am seeing those bugs and not so many other people is probably that I
> am using the menus. (Since I do not use Alt as META I can use them
> more easily. And I do.)

Perhaps you're right, but I don't believe you're the only heavy user
of menus. And still there aren't many menu-related bug reports. That
does not mean that there are no bugs, of course; just that they aren't
very frequent.

> I think you misunderstand the logic here. The race condition shows up
> because of system messages. In your case you probably see much, much
> less of those since (I believe) you use menus much less often.

I can believe that the messages help in making a race condition more
visible, but it should still happen every now and then without them.

> As I have explained quite a few times the refusal to add things like
> error checking puts a lot of burden on me.

There's no refusal to add error checking. There has been, several
times, a refusal to indiscriminately put such tests in places where
they don't seem necessary. But if you point out places where a system
call is likely to return an error code that we should check, nobody is
going to say "no, don't check that".

> You might want to fix the core Emacs.

I think that's a bit too vague to be of any help.

> Now I am a bit lost. I thought I had done that change, but I have not.
> (Or it has been reverted by some merge, that happens.)

What change?

> However I see another small change that I have made to w32term.c that
> might have fixed some crashes (but this is unrelated to the problem we
> are discussing) :
>
> *** 4491,4497 ****
>
>          else
>
>            {
>
>              f = x_window_to_frame (dpyinfo, msg.msg.hwnd);
>
> !             f->async_visible = msg.msg.wParam;
>
>            }
>
>  #endif

This patch is against your patched version, and in the trunk sources
the equivalent code is

	  /* If window has been obscured or exposed by another window
	     being maximised or minimised/restored, then recheck
	     visibility of all frames.  Direct changes to our own
	     windows get handled by WM_SIZE.  */
#if 0
	  if (msg.msg.lParam != 0)
	    check_visibility = 1;
	  else
	    {
	      f = x_window_to_frame (dpyinfo, msg.msg.hwnd);
	      f->async_visible = msg.msg.wParam;
	    }
#endif

which is obviously not executed.

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Mon, Jul 4, 2011 at 02:34, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, Jul 4, 2011 at 01:42, Lennart Borgman <lennart.borgman@gmail.com> wrote:
>
>> That there is (was) a bug in the menus is not hypothetical.
>
> That you have stumbled upon some bugs in the menus I don't deny. But
> this report was not about any bug report, just a system call return
> value that did not cause any bug (or at least, such bug was not
> reported).

This call was related to the menus.

>> The reason
>> I am seeing those bugs and not so many other people is probably that I
>> am using the menus. (Since I do not use Alt as META I can use them
>> more easily. And I do.)
>
> Perhaps you're right, but I don't believe you're the only heavy user
> of menus. And still there aren't many menu-related bug reports. That
> does not mean that there are no bugs, of course; just that they aren't
> very frequent.

I believe many of the heavy menu users might be using my patched
version (since this allows them to use the windows-key as META). That
might be the reason you do not see the bug reports.

>> I think you misunderstand the logic here. The race condition shows up
>> because of system messages. In your case you probably see much, much
>> less of those since (I believe) you use menus much less often.
>
> I can believe that the messages help in making a race condition more
> visible, but it should still happen every now and then without them.

Of course, but you just do not know it was a race condition. You have
no clue at all, since such a condition with a system call can give
very strange results. (I have seen such cases.)

>> As I have explained quite a few times the refusal to add things like
>> error checking puts a lot of burden on me.
>
> There's no refusal to add error checking. There has been, several
> times, a refusal to indiscriminately put such tests in places where
> they don't seem necessary. But if you point out places where a system
> call is likely to return an error code that we should check, nobody is
> going to say "no, don't check that".

I would say every system call. Why do you think some of them should be
excluded from error checking?

>> You might want to fix the core Emacs.
>
> I think that's a bit too vague to be of any help.

Hm. My mistake. ;-)

As I said I thought I had changed the code at that point I suggested,
in x_free_frame_resources. It was a simple change (and I still believe
it should be done).

>> Now I am a bit lost. I thought I had done that change, but I have not.
>> (Or it has been reverted by some merge, that happens.)
>
> What change?

That one in x_free_frame_resources (that I told about 2010-06-13).

>> However I see another small change that I have made to w32term.c that
>> might have fixed some crashes (but this is unrelated to the problem we
>> are discussing) :
>>
>> *** 4491,4497 ****
>>
>>          else
>>
>>            {
>>
>>              f = x_window_to_frame (dpyinfo, msg.msg.hwnd);
>>
>> !             f->async_visible = msg.msg.wParam;
>>
>>            }
>>
>>  #endif
>
> This patch is against your patched version, and in the trunk sources
> the equivalent code is
>
>          /* If window has been obscured or exposed by another window
>             being maximised or minimised/restored, then recheck
>             visibility of all frames.  Direct changes to our own
>             windows get handled by WM_SIZE.  */
> #if 0
>          if (msg.msg.lParam != 0)
>            check_visibility = 1;
>          else
>            {
>              f = x_window_to_frame (dpyinfo, msg.msg.hwnd);
>              f->async_visible = msg.msg.wParam;
>            }
> #endif
>
> which is obviously not executed.

:-)

I wonder why I cared to add that check then. Perhaps was it executed
before, I have no idea now.

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Mon, Jul 4, 2011 at 03:08, Lennart Borgman <lennart.borgman@gmail.com> wrote:

> This call was related to the menus.

This bug report was supposedly about menus, but was in fact about
system calls returning errors, and no menu bug in sight...

> I believe many of the heavy menu users might be using my patched
> version (since this allows them to use the windows-key as META). That
> might be the reason you do not see the bug reports.

Perhaps. It is hard to say.

> Of course, but you just do not know it was a race condition. You have
> no clue at all, since such a condition with a system call can give
> very strange results. (I have seen such cases.)

It's irrelevant whether the user knows that it was a race condition.
Either s/he sees a bug, or s/hee does not.

> I would say every system call. Why do you think some of them should be
> excluded from error checking?

There are many reasons. In some cases, an error means something could
not be done, but reporting it does not help and the fact that it was
not done does not cause any harm. In other cases, a system call can
return an error, but it just never happens (or if it happens, the
reason is serious enough that Emacs failing will not be the biggest
problem).

Adding error checking to all system calls adds unneeded complexity in
these cases where there's nothing to do if the system call fails, and
it failing is not going to cause data loss or the World War III.

> That one in x_free_frame_resources (that I told about 2010-06-13).

Again: what change? Adding error checking?

> I wonder why I cared to add that check then. Perhaps was it executed
> before, I have no idea now.

Then, why did you comment it?

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Mon, Jul 4, 2011 at 03:21, Juanma Barranquero <lekktu@gmail.com> wrote:
>
>> Of course, but you just do not know it was a race condition. You have
>> no clue at all, since such a condition with a system call can give
>> very strange results. (I have seen such cases.)
>
> It's irrelevant whether the user knows that it was a race condition.
> Either s/he sees a bug, or s/hee does not.

A debug message could still help.

>> I would say every system call. Why do you think some of them should be
>> excluded from error checking?
>
> There are many reasons. In some cases, an error means something could
> not be done, but reporting it does not help and the fact that it was
> not done does not cause any harm.

If that is taken care of I agree of course.

> In other cases, a system call can
> return an error, but it just never happens (or if it happens, the
> reason is serious enough that Emacs failing will not be the biggest
> problem).

A bad assumption in my book.

> Adding error checking to all system calls adds unneeded complexity in
> these cases where there's nothing to do if the system call fails, and
> it failing is not going to cause data loss or the World War III.

Yes, it is extra complexity. But I think the extra complexity that is
the result of not doing this error checking is much, much worse. There
might be very serious troubles and you have no idea what it is.

Note that not only errors in Emacs code can cause problems here, but
compiler bugs etc.

>> That one in x_free_frame_resources (that I told about 2010-06-13).
>
> Again: what change? Adding error checking?

Just switching the order:

  The problem seems to be in x_free_frame_resources. Should not
  free_frame_menubar be called before my_destroy_window there?

Hm. I am not quite sure now. Something in my memory is waving a flag
and saying something about it. Can't see it now ;-)

Could you please check the MS doc and see if these calls should be
switched? When I read them I came to that conclusion, but it might be
wrong (and the MS doc might also be wrong).

>> I wonder why I cared to add that check then. Perhaps was it executed
>> before, I have no idea now.
>
> Then, why did you comment it?

I did not notice now. Sorry.

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Mon, Jul 4, 2011 at 03:44, Lennart Borgman <lennart.borgman@gmail.com> wrote:

> A debug message could still help.

Yes. Or difficult debugging of other issues. Depends on the debug
message, its frequency and relevancy.

> A bad assumption in my book.

Well, we can disagree.

> Yes, it is extra complexity. But I think the extra complexity that is
> the result of not doing this error checking is much, much worse. There
> might be very serious troubles and you have no idea what it is.

If there are "serious troubles" and nobody ever notices them, then
they are not "serious troubles". We're talking about Emacs, not real
time avionics or nuclear plant control software.

> Could you please check the MS doc and see if these calls should be
> switched?

No. You're the one who's spent time looking at the menu code. You're
the one who should know whether that is really a bug and propose a
patch.

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Mon, Jul 4, 2011 at 04:13, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Mon, Jul 4, 2011 at 03:44, Lennart Borgman <lennart.borgman@gmail.com> wrote:
>
>> Yes, it is extra complexity. But I think the extra complexity that is
>> the result of not doing this error checking is much, much worse. There
>> might be very serious troubles and you have no idea what it is.
>
> If there are "serious troubles" and nobody ever notices them, then
> they are not "serious troubles". We're talking about Emacs, not real
> time avionics or nuclear plant control software.

There is serious trouble in this area. And if we are going to expand
the frame handling (for inline completion etc) then I think it will be
very good to have checks in all places.

However do as you want. I do not want to spent too much time on these
discussions. I think I have told my reasons for the checks.

>> Could you please check the MS doc and see if these calls should be
>> switched?
>
> No. You're the one who's spent time looking at the menu code. You're
> the one who should know whether that is really a bug and propose a
> patch.

OK. And what will happen when I have done that? (Which I have already done.)

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Mon, Jul 4, 2011 at 04:18, Lennart Borgman <lennart.borgman@gmail.com> wrote:

> OK. And what will happen when I have done that? (Which I have already done.)

The same as with any other patch by anyone else: you have to show that
the change is right and it's relly fixing a bug. And be willing to
rework the patch a few times if so requested by the people who
understands that part of Emacs.

What you cannot do (or, better, "what you should not do and expect to
get any sympathy") is to throw a patch here or to emacs-devel, reject
comments by knowledgeable people, and leave without following through
when some aspects of the patch are challenged with the argument that
you have much work and no time for this. And then say that your
patches are never accepted.

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lars Magne Ingebrigtsen]

Juanma Barranquero <lekktu@gmail.com> writes:

> The same as with any other patch by anyone else: you have to show that
> the change is right and it's relly fixing a bug. And be willing to
> rework the patch a few times if so requested by the people who
> understands that part of Emacs.

More information was requested more than a year ago, but was apparently
not given, so I'm closing this bug report.  If this still is a problem,
please reopen.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Wed, Sep 21, 2011 at 22:32, Lars Magne Ingebrigtsen <larsi@gnus.org> wrote:
> Juanma Barranquero <lekktu@gmail.com> writes:
>
>> The same as with any other patch by anyone else: you have to show that
>> the change is right and it's relly fixing a bug. And be willing to
>> rework the patch a few times if so requested by the people who
>> understands that part of Emacs.
>
> More information was requested more than a year ago, but was apparently
> not given, so I'm closing this bug report.  If this still is a problem,
> please reopen.

Closing this will not help. I think it should be left open!

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Juanma Barranquero]

On Wed, Sep 21, 2011 at 22:40, Lennart Borgman
<lennart.borgman@gmail.com> wrote:

> Closing this will not help. I think it should be left open!

Obviously, having it open has not helped either.

    Juanma

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Wed, Sep 21, 2011 at 22:44, Juanma Barranquero <lekktu@gmail.com> wrote:
> On Wed, Sep 21, 2011 at 22:40, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>
>> Closing this will not help. I think it should be left open!
>
> Obviously, having it open has not helped either.

Yes. It is not totally forgotten.

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lars Magne Ingebrigtsen]

Lennart Borgman <lennart.borgman@gmail.com> writes:

> Yes. It is not totally forgotten.

I don't think anybody understood what the bug report was even about.  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#6414: f->output_data.w32->menubar_widget uninitialized?

[Lennart Borgman]

On Wed, Sep 21, 2011 at 22:59, Lars Magne Ingebrigtsen <larsi@gnus.org> wrote:
> Lennart Borgman <lennart.borgman@gmail.com> writes:
>
>> Yes. It is not totally forgotten.
>
> I don't think anybody understood what the bug report was even about.  :-)

It is true that we do not understand where the problems are. If we did
we would have solved it ... ;-)

But that we do not understand is probably no good reason to close this one.