Date: Mon, 12 Mar 2012 02:30:15 GMT From: Mark Johnston <markjdb@gmail.com> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/143017: watch(8): fatal: cannot attach to tty Message-ID: <201203120230.q2C2UFEP058423@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/143017; it has been noted by GNATS. From: Mark Johnston <markjdb@gmail.com> To: Ed Schouten <ed@80386.nl> Cc: bug-followup@freebsd.org, dummy@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@80386.nl> wrote: > Hello Mark, > > * Mark <markjdb@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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201203120230.q2C2UFEP058423>