Re: System calls without error checks in w32

[Lennart Borgman]

On Mon, May 31, 2010 at 1:03 AM, Juanma Barranquero <address@hidden> wrote:
> On Mon, May 31, 2010 at 00:37, Lennart Borgman
> <address@hidden> wrote:
>
>> Yes, my intention was not to add them everywhere, only on interesting places.
>
> It didn't seem so in your first message.


Maybe not, but perhaps even more so from the reply ;-)


>> On a quick look I got the impression that file calls are better
>> handled than GUI system calls.
>
> Likely because file calls are much more likely to fail.


That is not what I think. I think instead that the consequences of
failing file calls are more visible and therefor much easier to catch.

And it looks to me from the code that the knowledge of other system
calls is less or at least more problematic to handle. At least on the
w32 side. And it is not surprising since we are few here looking at
that code. That is also one reason for adding debug calls since it can
give more knowledge of what is going on in that code.


>> It happens to me with my patched Emacs because that is the one I am
>> using. I only use unpatched Emacs for bug testing etc.
>
> Why don't you add these checks to your patched Emacs and see what comes of it?


It takes me quite a lot of time to take care of all the patches. There
does not seem to be much interest or time to fix many of the issues
and then I have to keep them in my patched version. I am trying to
minimize that work.

Also it is much more trouble making a patch later if I have a lot of
small patches that are not in the upstream Emacs.

I have wasted quite a lot of time on this.

One reason is of course that I do not install my patches myself. Maybe
it is time to do that instead. Is there any easy way out of the
situation that I have my checkout from Launchpad. Can I somehow make
bzr aware of that this is really the same thing as on savannah and
upload from it? Or should I make a new checkout from savannah and use
that just for this?


>> That is surely true, but I am not comfortable with how threads are
>> integrated (i.e. it is not visible enough IMO). I wonder if there are
>> any troubles there. Those threads of course communicate through system
>> calls.
>
> Do you wonder where "there are any troubles there", or do you suspect
> that there are troubles?


Both ;-)

How does blocking interfere with that code? Are the system calls
always done in the right thread (or with correct thread
communication)?


>> I might have fixed some of the problems with related to frames which
>> calls crashes, but there might be more.
>
> Well, perhaps you've fixed them, but as I've said, your Emacs seem to
> crash more often than most others ;-)


I think I have given the reason for that. So far I have never been
able to identify a crash as something depending on my code. And
believe it or not that is what I try to investigate first (which makes
my quite a bit angry about having to keep them myself)... ;-)


>> And I wonder why Emacs sometimes seems to hang temporarily when frame
>> related operations are going on. Is something going wrong?
>
> You'd have to be a bit more specific...


I see this when creating a frame in a timer. Or perhaps when fetching
info from the web in a timer. I am not sure, but none of these should
block.

Of course there must not be system calls involved directly. It might
be "bad chaining", i.e. code that is unnecessarily waiting for system
calls to complete.


>> And the inability to stop Emacs sometimes is quite disturbing. What is
>> going on there? Why does not the GUI thread get resources? Or is it
>> given resources but a block occur in some other way? Is GUI messages
>> not checked when looping?
>
> Also here.


Run something in a timer. Try to stop it.


>> This type of entering the checks is very
>> similar to what `assert' does in Emacs. Does that mean you think
>> `assert' is very ugly too or is there some difference I am missing?
>
> No, I quite like asserts. The difference is than assert is
>
>  assert (some test)
>
> There's nothing to understand there. It passes, or it fails.
> W32DEBPRINT has four arguments; at the very least, if would make more
> sense to do
>
>   W32DEBPRINT (this_function, ApiFunction (args) != bad_return_value)
>
> or somesuch, and, at that point, you're just reinventing assert, which
> seems pointless.


Oh, the four args where just to make it more easy to understand... ;-)

Do you mean there is an assert that can do DebPrint? That would be very nice.


> Anyway, my feeling is that it makes much more sense to check return
> codes *to do something* about them in the case of failure, that just
> to print a message which will be helpful or not, depending of how
> frequent / how informative they are.


Things sometimes does not go totally wrong with system calls even if
something is wrong because the system will try to recover. Looking at
the messages might then give some information.

The problem as I have seen it (it was a long time since I did
something like this) is often that you get too much information so you
have to shut down the logging and only have it on for short tests.
(That is one reason to keep the debug printing statements, but
commented out.)


> So if you're really interested in
> doing this, I'd suggest looking at specific unchecked syscalls and
> trying to determine if something (could / need to) be done in case of
> error. And then send patches ;-)


Sending patches is much easier if you do not have to remove a lot of
things from the diffs.