35291 2010-02-23 06:45:04 -0800 commit-log-editor can call itself in an infinite loop 2010-03-25 07:18:26 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P3 Minor --- 1 jchaffraix jchaffraix kenneth oldest_to_newest 192920 0 jchaffraix 2010-02-23 06:45:04 -0800 if $editor ends up being commit-log-editor, it will be called in an infinite loop. I managed to get this situation by swapping SVN_EDITOR and SVN_LOG_EDITOR - by mistake - in my .bashrc file. 192922 1 49287 jchaffraix 2010-02-23 06:49:35 -0800 Created attachment 49287 Proposed fix: add a isCommitLogEditor routine and check the $editor variable against it 194064 2 49287 kenneth 2010-02-26 05:18:46 -0800 Comment on attachment 49287 Proposed fix: add a isCommitLogEditor routine and check the $editor variable against it > +sub isCommitLogEditor($) > +{ > + my $editor = shift; > + printf $editor . "\n"; > + return $editor =~ m/commit-log-editor/; > +} Wouldn't it be enough to just do this check once and not for each time you call the subroutine? 194079 3 jchaffraix 2010-02-26 07:17:22 -0800 (In reply to comment #2) > (From update of attachment 49287 [details]) > > > +sub isCommitLogEditor($) > > +{ > > + my $editor = shift; > > + printf $editor . "\n"; > > + return $editor =~ m/commit-log-editor/; > > +} > > Wouldn't it be enough to just do this check once and not for each time you call > the subroutine? No, as you can't predict whether $editor is the same each time you are called. The use case for that would be setting $SVN_LOG_EDITOR to commit-log-editor. If you cache the result, then you will fall all the way down to "/usr/bin/vi" when you could have used $EDITOR. 201098 4 jchaffraix 2010-03-17 18:18:00 -0700 Kenneth, are you satisfied with my comment or would you prefer another approach? 201102 5 kenneth 2010-03-17 18:22:23 -0700 Yes! r=me 204040 6 jchaffraix 2010-03-25 07:18:26 -0700 Landed in r56540. Thanks! 49287 2010-02-23 06:49:35 -0800 2010-03-17 18:22:08 -0700 Proposed fix: add a isCommitLogEditor routine and check the $editor variable against it commit_log_editor.diff text/plain 2712 jchaffraix ZGlmZiAtLWdpdCBhL1dlYktpdFRvb2xzL0NoYW5nZUxvZyBiL1dlYktpdFRvb2xzL0NoYW5nZUxv ZwppbmRleCA1MzYyMDRiLi4xYjcyODJiIDEwMDY0NAotLS0gYS9XZWJLaXRUb29scy9DaGFuZ2VM b2cKKysrIGIvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTkgQEAKKzIwMTAtMDIt MjMgIEp1bGllbiBDaGFmZnJhaXggIDxqY2hhZmZyYWl4QHdlYmtpdC5vcmc+CisKKyAgICAgICAg UmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgY29tbWl0LWxvZy1lZGl0b3Ig Y2FuIGNhbGwgaXRzZWxmIGluIGFuIGluZmluaXRlIGxvb3AKKyAgICAgICAgaHR0cHM6Ly9idWdz LndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTM1MjkxCisKKyAgICAgICAgaWYgJGVkaXRvciBl bmRzIHVwIGJlaW5nIGNvbW1pdC1sb2ctZWRpdG9yLCB0aGUgc2NyaXB0IHdpbGwgZXhlYyBpdHNl bGYKKyAgICAgICAgaW4gYW4gaW5maW5pdGUgbG9vcC4KKworICAgICAgICBUbyBhdm9pZCB0aGlz LCB3ZSBub3cgY2hlY2sgdGhhdCB0aGUgJGVkaXRvciB2YXJpYWJsZSBpcyBub3QKKyAgICAgICAg Y29tbWl0LWxvZy1lZGl0b3IgdG8gYXZvaWQgdGhpcyBjYXNlLgorCisgICAgICAgICogU2NyaXB0 cy9jb21taXQtbG9nLWVkaXRvcjogQWRkZWQgYW4gaXNDb21taXRMb2dFZGl0b3IgbWV0aG9kIGFu ZAorICAgICAgICByZXdvcmtlZCB0aGUgJGVkaXRvciBzZXR0aW5nIHRvIGFkZCB0aGlzIGNoZWNr LgorCiAyMDEwLTAyLTIzICBTaGluaWNoaXJvIEhhbWFqaSAgPGhhbWFqaUBjaHJvbWl1bS5vcmc+ CiAKICAgICAgICAgUmV2aWV3ZWQgYnkgRXJpYyBTZWlkZWwuCmRpZmYgLS1naXQgYS9XZWJLaXRU b29scy9TY3JpcHRzL2NvbW1pdC1sb2ctZWRpdG9yIGIvV2ViS2l0VG9vbHMvU2NyaXB0cy9jb21t aXQtbG9nLWVkaXRvcgppbmRleCA3NTAxN2UzLi4xNDVhNThlIDEwMDc1NQotLS0gYS9XZWJLaXRU b29scy9TY3JpcHRzL2NvbW1pdC1sb2ctZWRpdG9yCisrKyBiL1dlYktpdFRvb2xzL1NjcmlwdHMv Y29tbWl0LWxvZy1lZGl0b3IKQEAgLTQwLDYgKzQwLDcgQEAgdXNlIHdlYmtpdGRpcnM7CiAKIHN1 YiBub3JtYWxpemVMaW5lRW5kaW5ncygkJCk7CiBzdWIgcmVtb3ZlTG9uZ2VzdENvbW1vblByZWZp eEVuZGluZ0luRG91YmxlTmV3bGluZShcJSk7CitzdWIgaXNDb21taXRMb2dFZGl0b3IoJCk7CiAK IHN1YiB1c2FnZQogewpAQCAtNjEsMjMgKzYyLDI2IEBAIGlmICghJGxvZykgewogbXkgJGJhc2VE aXIgPSBiYXNlUHJvZHVjdERpcigpOwogCiBteSAkZWRpdG9yID0gJEVOVntTVk5fTE9HX0VESVRP Un07Ci1pZiAoISRlZGl0b3IpIHsKK2lmICghJGVkaXRvciB8fCBpc0NvbW1pdExvZ0VkaXRvcigk ZWRpdG9yKSkgewogICAgICRlZGl0b3IgPSAkRU5We0NWU19MT0dfRURJVE9SfTsKIH0KLWlmICgh JGVkaXRvcikgeworaWYgKCEkZWRpdG9yIHx8IGlzQ29tbWl0TG9nRWRpdG9yKCRlZGl0b3IpKSB7 CiAgICAgbXkgJGJ1aWx0RWRpdG9yQXBwbGljYXRpb24gPSAiJGJhc2VEaXIvUmVsZWFzZS9Db21t aXQgTG9nIEVkaXRvci5hcHAvQ29udGVudHMvTWFjT1MvQ29tbWl0IExvZyBFZGl0b3IiOwogICAg ICRlZGl0b3IgPSAkYnVpbHRFZGl0b3JBcHBsaWNhdGlvbiBpZiAteCAkYnVpbHRFZGl0b3JBcHBs aWNhdGlvbjsKIH0KLWlmICghJGVkaXRvcikgeworaWYgKCEkZWRpdG9yIHx8IGlzQ29tbWl0TG9n RWRpdG9yKCRlZGl0b3IpKSB7CiAgICAgbXkgJGJ1aWx0RWRpdG9yQXBwbGljYXRpb24gPSAiJGJh c2VEaXIvRGVidWcvQ29tbWl0IExvZyBFZGl0b3IuYXBwL0NvbnRlbnRzL01hY09TL0NvbW1pdCBM b2cgRWRpdG9yIjsKICAgICAkZWRpdG9yID0gJGJ1aWx0RWRpdG9yQXBwbGljYXRpb24gaWYgLXgg JGJ1aWx0RWRpdG9yQXBwbGljYXRpb247CiB9Ci1pZiAoISRlZGl0b3IpIHsKK2lmICghJGVkaXRv ciB8fCBpc0NvbW1pdExvZ0VkaXRvcigkZWRpdG9yKSkgewogICAgIG15ICRpbnN0YWxsZWRFZGl0 b3JBcHBsaWNhdGlvbiA9ICIkRU5We0hPTUV9L0FwcGxpY2F0aW9ucy9Db21taXQgTG9nIEVkaXRv ci5hcHAvQ29udGVudHMvTWFjT1MvQ29tbWl0IExvZyBFZGl0b3IiOwogICAgICRlZGl0b3IgPSAk aW5zdGFsbGVkRWRpdG9yQXBwbGljYXRpb24gaWYgLXggJGluc3RhbGxlZEVkaXRvckFwcGxpY2F0 aW9uOwogfQotaWYgKCEkZWRpdG9yKSB7Ci0gICAgJGVkaXRvciA9ICRFTlZ7RURJVE9SfSB8fCAi L3Vzci9iaW4vdmkiOworaWYgKCEkZWRpdG9yIHx8IGlzQ29tbWl0TG9nRWRpdG9yKCRlZGl0b3Ip KSB7CisgICAgJGVkaXRvciA9ICRFTlZ7RURJVE9SfTsKK30KK2lmICghJGVkaXRvciB8fCBpc0Nv bW1pdExvZ0VkaXRvcigkZWRpdG9yKSkgeworICAgICRlZGl0b3IgPSAiL3Vzci9iaW4vdmkiOwog fQogCiBteSAkaW5DaGFuZ2VzVG9CZUNvbW1pdHRlZCA9ICFpc0dpdCgpOwpAQCAtMzA3LDMgKzMx MSwxMCBAQCBzdWIgcmVtb3ZlTG9uZ2VzdENvbW1vblByZWZpeEVuZGluZ0luRG91YmxlTmV3bGlu ZShcJSkKICAgICB9CiAgICAgcmV0dXJuIHN1YnN0cigkcHJlZml4LCAwLCAkbGFzdERvdWJsZU5l d2xpbmUgKyAyKTsKIH0KKworc3ViIGlzQ29tbWl0TG9nRWRpdG9yKCQpCit7CisgICAgbXkgJGVk aXRvciA9IHNoaWZ0OworICAgIHByaW50ZiAkZWRpdG9yIC4gIlxuIjsKKyAgICByZXR1cm4gJGVk aXRvciA9fiBtL2NvbW1pdC1sb2ctZWRpdG9yLzsKK30K