bin/143017: watch(8): fatal: cannot attach to tty

[Mark Johnston]

The following reply was made to PR bin/143017; it has been noted by GNATS.

From: Mark Johnston <markjdb at gmail.com>
To: Ed Schouten <ed at 80386.nl>
Cc: bug-followup at freebsd.org, dummy at smtp.ru
Subject: Re: bin/143017: watch(8): fatal: cannot attach to tty
Date: Sun, 11 Mar 2012 22:27:07 -0400

 --20cf307cfd988d304604bb027a31
 Content-Type: text/plain; charset=ISO-8859-1
 
 On Sat, Mar 10, 2012 at 9:59 AM, Ed Schouten <ed at 80386.nl> wrote:
 > Hello Mark,
 >
 > * Mark <markjdb at gmail.com>, 20120310 09:50:
 >> How about something like the attached change to snp(4)? It has snp
 >> detach from the tty when the input to SNPSTTY is -1, which is what the
 >> man page says is supposed to happen.
 >
 > This isn't the right approach. It would break when a snoop descriptor is
 > used by two or more threads. Imagine what would happen if one thread
 > would call read(), while the other would call SNPSTTY. The only way you
 > could implement this safely, is by locking down all the system calls
 > using for example an sx lock.
 
 Ah, I see. That didn't occur to me. As you say, it's probably better
 to fix this in watch(8), though I might try to clean up snp(4) at some
 point - there are a few FIXME comments, and the man page doesn't match
 the implementation... if you have any specific ideas on what should be
 done, let me know. =)
 
 > [...]
 > If someone would write a nice set of patches to clean up the watch(8)
 > source code (and fix this bug), I'd be more than happy to commit them to
 > SVN. In fact, I think we should simplify the watch(8) utility. In my
 > opinion the `interactive mode' is pretty useless.
 
 Could you take a look at the attached patch? It fixes the bug and does
 some other miscellaneous cleanup:
 - Merge open_snp() and attach_snp(), and change detch_snp() to close
 snp. Now the tty-switching code calls detach_snp() and set_dev(),
 which in turn calls attach_snp(). This also fixes bin/153052 and
 bin/153156 since we now don't try to open snp until after set_tty() is
 called.
 - Remove interactive mode... I don't see why it's needed either.
 - Remove an unnecessary "- 1" in a fgets() call.
 - Make sure at most one tty is listed on the command line.
 - Clean up the code that actually copies data from snp to stdout...
 what was there before seems kind of silly to me, but perhaps it should
 be left alone. Ditto for reading from stdin.
 
 There is probably more that can be done, but I thought this would be a
 good start. The only difference in behaviour is that watch(8) will
 clear the screen before trying to open snp(4) (which obviously might
 fail if snp isn't available or the user isn't root). I don't think
 this really a problem though.
 
 Thanks,
 -Mark
 
 --20cf307cfd988d304604bb027a31
 Content-Type: text/plain; charset=US-ASCII; name="watch_cleanup.patch.txt"
 Content-Disposition: attachment; filename="watch_cleanup.patch.txt"
 Content-Transfer-Encoding: base64
 X-Attachment-Id: f_gzow570x1
 
 ZGlmZiAtLWdpdCBhL3Vzci5zYmluL3dhdGNoL3dhdGNoLjggYi91c3Iuc2Jpbi93YXRjaC93YXRj
 aC44CmluZGV4IDk5YzZiNmYuLjc0NzQzZTkgMTAwNjQ0Ci0tLSBhL3Vzci5zYmluL3dhdGNoL3dh
 dGNoLjgKKysrIGIvdXNyLnNiaW4vd2F0Y2gvd2F0Y2guOApAQCAtNTMsMTMgKzUzLDYgQEAgV2l0
 aG91dCB0aGlzIG9wdGlvbiwKIHdpbGwgYXR0ZW1wdCB0byBmaW5kIHRoZSBuZXh0IGF2YWlsYWJs
 ZQogLlhyIHNucCA0CiBkZXZpY2UuCi0uSXQgRmwgaQotRm9yY2UgaW50ZXJhY3RpdmUgbW9kZS4K
 LUludGVyYWN0aXZlIG1vZGUgaXMgYSBkZWZhdWx0IGlmCi0uTm0KLWlzIHN0YXJ0ZWQgZnJvbSBh
 IHR0eS4KLUlmIG91dHB1dCBpcyByZWRpcmVjdGVkIHRvIGEgZmlsZSwgaW50ZXJhY3RpdmUgbW9k
 ZSBjYW4gc3RpbGwgYmUgcmVxdWVzdGVkCi1ieSBzcGVjaWZ5aW5nIHRoaXMgb3B0aW9uLgogLkl0
 IEZsIG4KIERpc2FibGUgdGhlIGFiaWxpdHkgdG8gc3dpdGNoIHRoZSB3YXRjaGVkIHR0eSBpbnRl
 cmFjdGl2ZWx5LgogVGhpcyBkaXNhYmxlcwpkaWZmIC0tZ2l0IGEvdXNyLnNiaW4vd2F0Y2gvd2F0
 Y2guYyBiL3Vzci5zYmluL3dhdGNoL3dhdGNoLmMKaW5kZXggZTY5NTM0ZC4uZDEzODJiOCAxMDA2
 NDQKLS0tIGEvdXNyLnNiaW4vd2F0Y2gvd2F0Y2guYworKysgYi91c3Iuc2Jpbi93YXRjaC93YXRj
 aC5jCkBAIC00NSw3ICs0NSw3IEBAIF9fRkJTRElEKCIkRnJlZUJTRCQiKTsKICNkZWZpbmUgTVNH
 X05PV1JJVEUJIlNub29wIGRldmljZSBjaGFuZ2UgZHVlIHRvIHdyaXRlIGZhaWx1cmUuIgogCiAj
 ZGVmaW5lIERFVl9OQU1FX0xFTgkxMDI0CS8qIGZvciAvZGV2L3R0eVhYKysgKi8KLSNkZWZpbmUg
 TUlOX1NJWkUJMjU2CisjZGVmaW5lIEJVRl9TSVpFCTI1NgogCiAjZGVmaW5lIENIUl9TV0lUQ0gJ
 MjQJLyogQ3RybCtYCSAqLwogI2RlZmluZSBDSFJfQ0xFQVIJMjMJLyogQ3RybCtWCSAqLwpAQCAt
 NTUsNyArNTUsNiBAQCBzdGF0aWMgdm9pZAl0aW1lc3RhbXAoY29uc3QgY2hhciAqKTsKIHN0YXRp
 YyB2b2lkCXNldF90dHkodm9pZCk7CiBzdGF0aWMgdm9pZAl1bnNldF90dHkodm9pZCk7CiBzdGF0
 aWMgdm9pZAlmYXRhbChpbnQsIGNvbnN0IGNoYXIgKik7Ci1zdGF0aWMgaW50CW9wZW5fc25wKHZv
 aWQpOwogc3RhdGljIHZvaWQJY2xlYW51cChpbnQpOwogc3RhdGljIHZvaWQJdXNhZ2Uodm9pZCkg
 X19kZWFkMjsKIHN0YXRpYyB2b2lkCXNldHVwX3Njcih2b2lkKTsKQEAgLTE0OSwyNiArMTQ4LDYg
 QEAgZmF0YWwoaW50IGVycm9yLCBjb25zdCBjaGFyICpidWYpCiAJCWV4aXQoZXJyb3IpOwogfQog
 Ci1zdGF0aWMgaW50Ci1vcGVuX3NucCh2b2lkKQotewotCWludAkJZiwgbW9kZTsKLQotCWlmIChv
 cHRfd3JpdGUpCi0JCW1vZGUgPSBPX1JEV1I7Ci0JZWxzZQotCQltb2RlID0gT19SRE9OTFk7Ci0K
 LQlpZiAob3B0X3NucGRldiA9PSBOVUxMKQotCQlmID0gb3BlbihfUEFUSF9ERVYgInNucCIsIG1v
 ZGUpOwotCWVsc2UKLQkJZiA9IG9wZW4ob3B0X3NucGRldiwgbW9kZSk7Ci0JaWYgKGYgPT0gLTEp
 Ci0JCWZhdGFsKEVYX09TRklMRSwgImNhbm5vdCBvcGVuIHNub29wIGRldmljZSIpOwotCi0JcmV0
 dXJuIChmKTsKLX0KLQogc3RhdGljIHZvaWQKIGNsZWFudXAoaW50IHNpZ25vIF9fdW51c2VkKQog
 ewpAQCAtMjA2LDI1ICsxODUsMzMgQEAgc2V0dXBfc2NyKHZvaWQpCiBzdGF0aWMgdm9pZAogZGV0
 YWNoX3NucCh2b2lkKQogewotCWludAkJZmQ7CiAKLQlmZCA9IC0xOwotCWlvY3RsKHNucF9pbywg
 U05QU1RUWSwgJmZkKTsKKwljbG9zZShzbnBfaW8pOwogfQogCiBzdGF0aWMgdm9pZAogYXR0YWNo
 X3NucCh2b2lkKQogewotCWludAkJc25wX3R0eTsKKwlpbnQgc25wX3R0eSwgbW9kZTsKKworCWlm
 IChvcHRfd3JpdGUpCisJCW1vZGUgPSBPX1JEV1I7CisJZWxzZQorCQltb2RlID0gT19SRE9OTFk7
 CisKKwlpZiAob3B0X3NucGRldiA9PSBOVUxMKQorCQlzbnBfaW8gPSBvcGVuKF9QQVRIX0RFViAi
 c25wIiwgbW9kZSk7CisJZWxzZQorCQlzbnBfaW8gPSBvcGVuKG9wdF9zbnBkZXYsIG1vZGUpOwor
 CWlmIChzbnBfaW8gPT0gLTEpCisJCWZhdGFsKEVYX09TRklMRSwgImNhbm5vdCBvcGVuIHNub29w
 IGRldmljZSIpOwogCiAJc25wX3R0eSA9IG9wZW4oZGV2X25hbWUsIE9fUkRPTkxZIHwgT19OT05C
 TE9DSyk7Ci0JaWYgKHNucF90dHkgPCAwKQorCWlmIChzbnBfdHR5ID09IC0xKQogCQlmYXRhbChF
 WF9EQVRBRVJSLCAiY2FuJ3Qgb3BlbiBkZXZpY2UiKTsKIAlpZiAoaW9jdGwoc25wX2lvLCBTTlBT
 VFRZLCAmc25wX3R0eSkgIT0gMCkKIAkJZmF0YWwoRVhfVU5BVkFJTEFCTEUsICJjYW5ub3QgYXR0
 YWNoIHRvIHR0eSIpOwogCWNsb3NlKHNucF90dHkpOwotCWlmIChvcHRfdGltZXN0YW1wKQotCQl0
 aW1lc3RhbXAoIkxvZ2dpbmcgU3RhcnRlZC4iKTsKIH0KIAogc3RhdGljIHZvaWQKQEAgLTI2OSw3
 ICsyNTYsNyBAQCBhc2tfZGV2KGNoYXIgKmRidWYsIGNvbnN0IGNoYXIgKm1zZykKIAllbHNlCiAJ
 CXByaW50ZigiRW50ZXIgZGV2aWNlIG5hbWU6Iik7CiAKLQlpZiAoZmdldHMoYnVmLCBERVZfTkFN
 RV9MRU4gLSAxLCBzdGRpbikpIHsKKwlpZiAoZmdldHMoYnVmLCBERVZfTkFNRV9MRU4sIHN0ZGlu
 KSkgewogCQlsZW4gPSBzdHJsZW4oYnVmKTsKIAkJaWYgKGJ1ZltsZW4gLSAxXSA9PSAnXG4nKQog
 CQkJYnVmW2xlbiAtIDFdID0gJ1wwJzsKQEAgLTI4NSw4ICsyNzIsNyBAQCBpbnQKIG1haW4oaW50
 IGFjLCBjaGFyICphdltdKQogewogCWludAkJY2gsIHJlcywgcnYsIG5yZWFkOwotCXNpemVfdAkJ
 Yl9zaXplID0gTUlOX1NJWkU7Ci0JY2hhcgkJKmJ1ZiwgY2hiW1JFQURCX0xFTl07CisJY2hhcgkJ
 YnVmW0JVRl9TSVpFXSwgY2hiW1JFQURCX0xFTl07CiAJZmRfc2V0CQlmZF9zOwogCiAJKHZvaWQp
 IHNldGxvY2FsZShMQ19USU1FLCAiIik7CkBAIC0yOTYsNyArMjgyLDcgQEAgbWFpbihpbnQgYWMs
 IGNoYXIgKmF2W10pCiAJZWxzZQogCQlvcHRfaW50ZXJhY3RpdmUgPSAwOwogCi0Jd2hpbGUgKChj
 aCA9IGdldG9wdChhYywgYXYsICJXY2lvdG5mOiIpKSAhPSAtMSkKKwl3aGlsZSAoKGNoID0gZ2V0
 b3B0KGFjLCBhdiwgIldjb3RuZjoiKSkgIT0gLTEpCiAJCXN3aXRjaCAoY2gpIHsKIAkJY2FzZSAn
 Vyc6CiAJCQlvcHRfd3JpdGUgPSAxOwpAQCAtMzA0LDkgKzI5MCw2IEBAIG1haW4oaW50IGFjLCBj
 aGFyICphdltdKQogCQljYXNlICdjJzoKIAkJCW9wdF9yZWNvbm5fY2xvc2UgPSAxOwogCQkJYnJl
 YWs7Ci0JCWNhc2UgJ2knOgotCQkJb3B0X2ludGVyYWN0aXZlID0gMTsKLQkJCWJyZWFrOwogCQlj
 YXNlICdvJzoKIAkJCW9wdF9yZWNvbm5fb2Zsb3cgPSAxOwogCQkJYnJlYWs7CkBAIC0zMjQsMjcg
 KzMwNywyOSBAQCBtYWluKGludCBhYywgY2hhciAqYXZbXSkKIAkJCXVzYWdlKCk7CiAJCX0KIAor
 CWlmIChhYyAtIG9wdGluZCA+IDEpCisJCXVzYWdlKCk7CisKIAlpZiAobW9kZmluZCgic25wIikg
 PT0gLTEpCiAJCWlmIChrbGRsb2FkKCJzbnAiKSA9PSAtMSB8fCBtb2RmaW5kKCJzbnAiKSA9PSAt
 MSkKIAkJCXdhcm4oInNucCBtb2R1bGUgbm90IGF2YWlsYWJsZSIpOwogCiAJc2lnbmFsKFNJR0lO
 VCwgY2xlYW51cCk7CiAKLQlzbnBfaW8gPSBvcGVuX3NucCgpOwogCXNldHVwX3NjcigpOwogCiAJ
 aWYgKCooYXYgKz0gb3B0aW5kKSA9PSBOVUxMKSB7CiAJCWlmIChvcHRfaW50ZXJhY3RpdmUgJiYg
 IW9wdF9ub19zd2l0Y2gpCiAJCQlhc2tfZGV2KGRldl9uYW1lLCBNU0dfSU5JVCk7CiAJCWVsc2UK
 LQkJCWZhdGFsKEVYX0RBVEFFUlIsICJubyBkZXZpY2UgbmFtZSBnaXZlbiIpOworCQkJZXJyeChF
 WF9EQVRBRVJSLCAibm8gZGV2aWNlIG5hbWUgZ2l2ZW4iKTsKIAl9IGVsc2UKIAkJc3RybmNweShk
 ZXZfbmFtZSwgKmF2LCBERVZfTkFNRV9MRU4pOwogCiAJc2V0X2RldihkZXZfbmFtZSk7CiAKLQlp
 ZiAoIShidWYgPSAoY2hhciAqKSBtYWxsb2MoYl9zaXplKSkpCi0JCWZhdGFsKEVYX1VOQVZBSUxB
 QkxFLCAibWFsbG9jIGZhaWxlZCIpOworCWlmIChvcHRfdGltZXN0YW1wKQorCQl0aW1lc3RhbXAo
 IkxvZ2dpbmcgU3RhcnRlZC4iKTsKIAogCUZEX1pFUk8oJmZkX3MpOwogCkBAIC0zNTQsMTMgKzMz
 OSw4IEBAIG1haW4oaW50IGFjLCBjaGFyICphdltdKQogCQlGRF9TRVQoc25wX2lvLCAmZmRfcyk7
 CiAJCXJlcyA9IHNlbGVjdChzbnBfaW8gKyAxLCAmZmRfcywgTlVMTCwgTlVMTCwgTlVMTCk7CiAJ
 CWlmIChvcHRfaW50ZXJhY3RpdmUgJiYgRkRfSVNTRVQoc3RkX2luLCAmZmRfcykpIHsKLQotCQkJ
 aWYgKChyZXMgPSBpb2N0bChzdGRfaW4sIEZJT05SRUFELCAmbnJlYWQpKSAhPSAwKQotCQkJCWZh
 dGFsKEVYX09TRVJSLCAiaW9jdGwoRklPTlJFQUQpIik7Ci0JCQlpZiAobnJlYWQgPiBSRUFEQl9M
 RU4pCi0JCQkJbnJlYWQgPSBSRUFEQl9MRU47Ci0JCQlydiA9IHJlYWQoc3RkX2luLCBjaGIsIG5y
 ZWFkKTsKLQkJCWlmIChydiA9PSAtMSB8fCBydiAhPSBucmVhZCkKKwkJCXJ2ID0gcmVhZChzdGRf
 aW4sIGNoYiwgUkVBREJfTEVOKTsKKwkJCWlmIChydiA9PSAtMSkKIAkJCQlmYXRhbChFWF9JT0VS
 UiwgInJlYWQgKHN0ZGluKSBmYWlsZWQiKTsKIAogCQkJc3dpdGNoIChjaGJbMF0pIHsKQEAgLTM3
 Niw4ICszNTYsOCBAQCBtYWluKGludCBhYywgY2hhciAqYXZbXSkKIAkJCQl9CiAJCQlkZWZhdWx0
 OgogCQkJCWlmIChvcHRfd3JpdGUpIHsKLQkJCQkJcnYgPSB3cml0ZShzbnBfaW8sIGNoYiwgbnJl
 YWQpOwotCQkJCQlpZiAocnYgPT0gLTEgfHwgcnYgIT0gbnJlYWQpIHsKKwkJCQkJcnYgPSB3cml0
 ZShzbnBfaW8sIGNoYiwgcnYpOworCQkJCQlpZiAocnYgPT0gLTEpIHsKIAkJCQkJCWRldGFjaF9z
 bnAoKTsKIAkJCQkJCWlmIChvcHRfbm9fc3dpdGNoKQogCQkJCQkJCWZhdGFsKEVYX0lPRVJSLApA
 QCAtMzkyLDcgKzM3Miw3IEBAIG1haW4oaW50IGFjLCBjaGFyICphdltdKQogCQlpZiAoIUZEX0lT
 U0VUKHNucF9pbywgJmZkX3MpKQogCQkJY29udGludWU7CiAKLQkJaWYgKChyZXMgPSBpb2N0bChz
 bnBfaW8sIEZJT05SRUFELCAmbnJlYWQpKSAhPSAwKQorCQlpZiAoaW9jdGwoc25wX2lvLCBGSU9O
 UkVBRCwgJm5yZWFkKSAhPSAwKQogCQkJZmF0YWwoRVhfT1NFUlIsICJpb2N0bChGSU9OUkVBRCki
 KTsKIAogCQlzd2l0Y2ggKG5yZWFkKSB7CkBAIC00MTYsMjMgKzM5NiwxMSBAQCBtYWluKGludCBh
 YywgY2hhciAqYXZbXSkKIAkJCQljbGVhbnVwKC0xKTsKIAkJCWJyZWFrOwogCQlkZWZhdWx0Ogot
 CQkJaWYgKG5yZWFkIDwgKGJfc2l6ZSAvIDIpICYmIChiX3NpemUgLyAyKSA+IE1JTl9TSVpFKSB7
 Ci0JCQkJZnJlZShidWYpOwotCQkJCWlmICghKGJ1ZiA9IChjaGFyICopIG1hbGxvYyhiX3NpemUg
 LyAyKSkpCi0JCQkJCWZhdGFsKEVYX1VOQVZBSUxBQkxFLCAibWFsbG9jIGZhaWxlZCIpOwotCQkJ
 CWJfc2l6ZSA9IGJfc2l6ZSAvIDI7Ci0JCQl9Ci0JCQlpZiAobnJlYWQgPiBiX3NpemUpIHsKLQkJ
 CQliX3NpemUgPSAobnJlYWQgJSAyKSA/IChucmVhZCArIDEpIDogKG5yZWFkKTsKLQkJCQlmcmVl
 KGJ1Zik7Ci0JCQkJaWYgKCEoYnVmID0gKGNoYXIgKikgbWFsbG9jKGJfc2l6ZSkpKQotCQkJCQlm
 YXRhbChFWF9VTkFWQUlMQUJMRSwgIm1hbGxvYyBmYWlsZWQiKTsKLQkJCX0KLQkJCXJ2ID0gcmVh
 ZChzbnBfaW8sIGJ1ZiwgbnJlYWQpOwotCQkJaWYgKHJ2ID09IC0xIHx8IHJ2ICE9IG5yZWFkKQor
 CQkJcnYgPSByZWFkKHNucF9pbywgYnVmLCBCVUZfU0laRSk7CisJCQlpZiAocnYgPT0gLTEpCiAJ
 CQkJZmF0YWwoRVhfSU9FUlIsICJyZWFkIGZhaWxlZCIpOwotCQkJcnYgPSB3cml0ZShzdGRfb3V0
 LCBidWYsIG5yZWFkKTsKLQkJCWlmIChydiA9PSAtMSB8fCBydiAhPSBucmVhZCkKKwkJCXJ2ID0g
 d3JpdGUoc3RkX291dCwgYnVmLCBydik7CisJCQlpZiAocnYgPT0gLTEpCiAJCQkJZmF0YWwoRVhf
 SU9FUlIsICJ3cml0ZSBmYWlsZWQiKTsKIAkJfQogCX0JCQkvKiBXaGlsZSAqLwo=
 --20cf307cfd988d304604bb027a31--