Windows patch status?

Windows patch status?

[Mark Mitchell]

Mark --

After our last discussion, I posted a revised patch here:

   http://sources.redhat.com/ml/gdb-patches/2005-04/msg00054.html

I believe that version reflects the improvements you requested.

Have you had a chance to look at it?

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: Windows patch status?

[Mark Kettenis]

   Date: Mon, 18 Apr 2005 08:05:12 -0700
   From: Mark Mitchell <mark@codesourcery.com>

   Mark --

   After our last discussion, I posted a revised patch here:

      http://sources.redhat.com/ml/gdb-patches/2005-04/msg00054.html

   I believe that version reflects the improvements you requested.

   Have you had a chance to look at it?

Oops.  Missed it again.  I looked at it now.

   It's possible to "#define ioctl ioctlsocket" so that ioctl doesn't
   need to change. However, I think it would be very confusing to do
   "#define close closesocket" because there's already a Windows
   runtime library function called "close" -- it just doesn't work on
   sockets. So, in the version of the patch that's attached, we still
   do have a call to closesocket.

But the code only uses close for sockets isn't it?  The point I'm
trying to make is that most people changing this file will be
familliar with POSIX but not with the stupid Windows API.  I really
want this file to look as much as POSIX as possible.  Given that this
file is all about sockets, I think the #define close closesocket might
actually prevent people who are not familliar with windows from using
close() where they should use closesocket().  So I'd still like you to
change this.


+   delta = (timeout == 0 ? 0 : 1);
+   while (1)
+     {
+ 
+       /* N.B. The UI may destroy our world (for instance by calling
+          remote_stop,) in which case we want to get out of here as
+          quickly as possible.  It is not safe to touch scb, since

Can you remove the extra blank line here?


+       /* If we got a character or an error back from wait_for, then we can 
+          break from the loop before the timeout is completed. */
+ 
+       if (status != SERIAL_TIMEOUT)
+ 	{
+ 	  break;
+ 	}

and here (and get rid of the redundant braces.

+       /* If we have exhausted the original timeout, then generate
+          a SERIAL_TIMEOUT, and pass it out of the loop. */
+ 
+       else if (timeout == 0)
+ 	{
+ 	  status = SERIAL_TIMEOUT;

and get rid of that blank line too.

+   if (status <= 0)
+     {
+       if (status == 0)
+ 	return SERIAL_TIMEOUT;	/* 0 chars means timeout [may need to
+ 				   distinguish between EOF & timeouts
+ 				   someday] */
+       else
+ 	return SERIAL_ERROR;	/* Got an error from read */
+     }

Please give these long comments their own line, and make them full
sentences (full stop and two spaces at the end, you know the drill).

With those changes, please go ahead and check this in.

Mark

Re: Windows patch status?

[Mark Mitchell]

Mark Kettenis wrote:
>    Date: Mon, 18 Apr 2005 08:05:12 -0700
>    From: Mark Mitchell <mark@codesourcery.com>
> 
>    Mark --
> 
>    After our last discussion, I posted a revised patch here:
> 
>       http://sources.redhat.com/ml/gdb-patches/2005-04/msg00054.html
> 
>    I believe that version reflects the improvements you requested.
> 
>    Have you had a chance to look at it?
> 
> Oops.  Missed it again.  I looked at it now.

I made the changes you requested, and checked it in.

There's only one more patch -- I'll be preparing that for posting shortly.

Thanks for your help.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304