Yitzchak Scott-Thoennes | 1 Aug 2005 13:15
Favicon
Gravatar

[PATCH] TIOCMBI[SC]

I don't have a serial device to test this with, but it's just selected
parts of the TIOCMSET handling slightly adapted.

2005-08-01  Yitzchak Scott-Thoennes  <sthoenna <at> efn.org>

	* fhandler_serial.cc (fhandler_serial::ioctl): Implement TIOCMBIS and
	TIOCMBIC.
	* include/sys/termios.h: Define TIOCMBIS and TIOCMBIC

.
--- winsup/cygwin/include/sys/termios.h.orig	2005-05-01 20:50:10.000000000 -0700
+++ winsup/cygwin/include/sys/termios.h	2005-08-01 02:22:34.361969600 -0700
 <at>  <at>  -1,6 +1,6  <at>  <at> 
 /* sys/termios.h

-   Copyright 1997, 1998, 1999, 2000, 2001, 2002, 2003 Red Hat, Inc.
+   Copyright 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2005 Red Hat, Inc.

 This file is part of Cygwin.

 <at>  <at>  -14,6 +14,8  <at>  <at> 
 #define _SYS_TERMIOS_H

 #define	TIOCMGET	0x5415
+#define	TIOCMBIS	0x5416
+#define	TIOCMBIC	0x5417
 #define	TIOCMSET	0x5418
 #define	TIOCINQ		0x541B
(Continue reading)

Corinna Vinschen | 1 Aug 2005 18:50
Favicon

Re: fix possible segfault creating detached thread

On Jul 31 15:17, Mike Gorse wrote:
> This patch fixes a seg fault when a thread is created in a detached state 
> and terminates the first time it is scheduled.  pthread::create (the 
> four-parameter version) calls the three-parameter pthread::create function 
> which unlocks the mutex, allowing the called thread to be scheduled, then 
> exits at which point the outer create function calls is_good_objectg(), 
> but this causes a core dump if pthread::exit() has already been called and 
> deleted the pthread object.

Thanks for the patch.  First, please let me point you to
http://cygwin.com/contrib.html.  The important information here is that
you'll need to fill out a copyright assignment form and snail mail it
to Red Hat if you want to get in patches.  The only exception are 
insignificant patches in terms of changed lines of code.  The usual rule of
thumb here is not more than 10 lines.  Well, your patch only changes
roughly 12 lines, so I'd let slip it in.

However, there are three tiny problems:

> 2005-07-31 Michael Gorse <mgorse <at> alum.wpi.edu>
> 
>         * thread.cc (pthread::create): Make bool.
>         * thread.cc (pthread_null::create): Ditto.

Wrong ChangeLog entry format.  Don't repeat the name of the file for each
change.  Compare with the current ChangeLog file.  And don't add an empty
line for each changed file, unless the changes are unrelated.

>         * thread.h: Ditto.
> 
(Continue reading)

Corinna Vinschen | 1 Aug 2005 18:56
Favicon

Re: [PATCH] TIOCMBI[SC]

On Aug  1 04:15, Yitzchak Scott-Thoennes wrote:
> I don't have a serial device to test this with, but it's just selected
> parts of the TIOCMSET handling slightly adapted.

I'm not serial I/O savvy, but the change looks pretty much ok.  I'm just
not exactly glad that the functionality itself is duplicated.  Would you
mind a rewrite so that the functionality is not copied, for instance by
creating a private method which does it, or by recursively calling
fhandler_serial::ioctl() with tweaked arguments (TIOCMSET)?

Corinna

--

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          mailto:cygwin <at> cygwin.com
Red Hat, Inc.

Corinna Vinschen | 2 Aug 2005 11:35
Favicon

Re: fix possible segfault creating detached thread

On Aug  1 20:05, Mike Gorse wrote:
> ARGH!  I really need to be more careful...  Sorry for all the emails.
> 
> One last correction...

> 2005-08-01 Michael Gorse <mgorse <at> alum.wpi.edu>
> 
>         * thread.cc (pthread::create(3 args)): Make bool.
>         (pthread_null::create): Ditto.
>         thread.h: Ditto.
> 
>         * thread.cc (pthread::create(4 args)): Check return of inner
>         create  rather than calling is_good_object().

Nope, sorry.  It should look like this:

2005-08-01 Michael Gorse <mgorse <at> alum.wpi.edu>

        * thread.cc (pthread::create(3 args)): Make bool.
        (pthread_null::create): Ditto.
        (pthread::create(4 args)): Check return of inner create rather than
	calling is_good_object().
        * thread.h: Ditto.

Can you please review your patch file?  I was unable to apply the patch,
even when using the -l option:

$ patch -l -p0 < ~/thread.diff
patching file thread.cc
Hunk #1 FAILED at 491.
(Continue reading)

Yitzchak Scott-Thoennes | 1 Aug 2005 21:25
Favicon
Gravatar

Re: [PATCH] TIOCMBI[SC]

On Mon, Aug 01, 2005 at 06:56:39PM +0200, Corinna Vinschen wrote:
> On Aug  1 04:15, Yitzchak Scott-Thoennes wrote:
> > I don't have a serial device to test this with, but it's just selected
> > parts of the TIOCMSET handling slightly adapted.
> 
> I'm not serial I/O savvy, but the change looks pretty much ok.  I'm just
> not exactly glad that the functionality itself is duplicated.  Would you
> mind a rewrite so that the functionality is not copied, for instance by
> creating a private method which does it, or by recursively calling
> fhandler_serial::ioctl() with tweaked arguments (TIOCMSET)?

No problem.  How does this look?

2005-08-01  Yitzchak Scott-Thoennes  <sthoenna <at> efn.org>

	* include/sys/termios.h: Define TIOCMBIS and TIOCMBIC.
        * fhandler.h (class fhandler_serial): Declare switch_modem_lines.
	* fhandler_serial.cc (fhandler_serial::switch_modem_lines): New
        static function to set or clear DTR and/or RTS.
        (fhandler_serial::ioctl): Use switch_modem_lines for TIOCMSET
        and new TIOCMBIS and TIOCMBIC.

--- winsup/cygwin/include/sys/termios.h.orig	2005-05-01 20:50:10.000000000 -0700
+++ winsup/cygwin/include/sys/termios.h	2005-08-01 02:22:34.361969600 -0700
 <at>  <at>  -1,6 +1,6  <at>  <at> 
 /* sys/termios.h

-   Copyright 1997, 1998, 1999, 2000, 2001, 2002, 2003 Red Hat, Inc.
(Continue reading)

Mike Gorse | 5 Aug 2005 14:31
Favicon

Re: fix possible segfault creating detached thread (fwd)

On Tue, 2 Aug 2005, Corinna Vinschen wrote:

> Can you please review your patch file?  I was unable to apply the patch,
> even when using the -l option:

Pine must be wrapping lines.  I'm resending it as an attachment.

2005-08-05 Michael Gorse <mgorse <at> alum.wpi.edu>

  * thread.cc (pthread::create(3 args)): Make bool.
  (pthread_null::create): Ditto.
  (pthread::create(4 args)): Check return of inner create rather than
  calling is_good_object().
  * thread.h: Ditto.
Index: thread.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/thread.cc,v
retrieving revision 1.190
diff -u -p -r1.190 thread.cc
--- thread.cc	6 Jul 2005 20:05:03 -0000	1.190
+++ thread.cc	2 Aug 2005 13:15:04 -0000
 <at>  <at>  -491,13 +491,15  <at>  <at>  pthread::precreate (pthread_attr *newatt
     magic = 0;
 }
 
-void
+bool
 pthread::create (void *(*func) (void *), pthread_attr *newattr,
 		 void *threadarg)
(Continue reading)

Vaclav Haisman | 5 Aug 2005 15:01
Picon
Picon

fhandler_tty_slave::tcflush() in fhandler_tty.cc

fhandler_tty_slave::tcflush() is IMHO still wrong. The result of comparison 
is bool and bool converted to int is either 1 or 0. The following patch 
should cure it.

VH

2005-08-05  Vaclav Haisman  <v.haisman <at> sh.cvut.cz>

 * fhandler_tty.cc (fhandler_tty_slave::tcflush): Return either 0
 or -1.
Attachment (fhandler_tty.cc.diff): application/octet-stream, 565 bytes
Dave Korn | 5 Aug 2005 16:17
Favicon

RE: fhandler_tty_slave::tcflush() in fhandler_tty.cc

----Original Message----
>From: Vaclav Haisman
>Sent: 05 August 2005 14:01

> fhandler_tty_slave::tcflush() is IMHO still wrong. The result of
> comparison is bool and bool converted to int is either 1 or 0. The
> following patch should cure it.

  Just to enlarge upon that, the problem is not just that it's returning a
zero-or-one when it should be returning a zero-or-minus-one result, but that
the logical sense is inverted too: when flushing input, the test means that
it used to return zero for failure and non-zero for success.  Your patch
looks good to me.

    cheers,
      DaveK
--

-- 
Can't think of a witty .sigline today....

Corinna Vinschen | 5 Aug 2005 18:11
Favicon

Re: fhandler_tty_slave::tcflush() in fhandler_tty.cc

On Aug  5 15:01, Vaclav Haisman wrote:
> 2005-08-05  Vaclav Haisman  <v.haisman <at> sh.cvut.cz>
> 
> * fhandler_tty.cc (fhandler_tty_slave::tcflush): Return either 0
> or -1.

Thanks,
applied.

Corinna

--

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          mailto:cygwin <at> cygwin.com
Red Hat, Inc.

Corinna Vinschen | 5 Aug 2005 18:16
Favicon

Re: fix possible segfault creating detached thread (fwd)

On Aug  5 08:31, Mike Gorse wrote:
> 2005-08-05 Michael Gorse <mgorse <at> alum.wpi.edu>
> 
>  * thread.cc (pthread::create(3 args)): Make bool.
>  (pthread_null::create): Ditto.
>  (pthread::create(4 args)): Check return of inner create rather than
>  calling is_good_object().
>  * thread.h: Ditto.

Thanks, applied.

Corinna

--

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          mailto:cygwin <at> cygwin.com
Red Hat, Inc.


Gmane