83544 2012-04-09 20:55:44 -0700 Make CSSParser::parseValue()'s handling of CSSPropertyCursor more obviously correct. 2012-04-17 15:56:46 -0700 1 1 1 Unclassified WebKit CSS 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 macpherson macpherson abarth darin haraken macpherson menard mikelawther webkit.review.bot oldest_to_newest 598967 0 macpherson 2012-04-09 20:55:44 -0700 Make CSSParser::parseValue()'s handling of CSSPropertyCursor more obviously correct. 598970 1 136389 macpherson 2012-04-09 20:59:31 -0700 Created attachment 136389 Patch 599158 2 136389 menard 2012-04-10 04:33:48 -0700 Comment on attachment 136389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136389&action=review > Source/WebCore/css/CSSParser.cpp:1595 > + if (value) { Why you assert here if you check with an if? 599163 3 136389 haraken 2012-04-10 05:11:33 -0700 Comment on attachment 136389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136389&action=review > Source/WebCore/css/CSSParser.cpp:1581 > + ASSERT(list || value); I am not sure what the ASSERT() is for. It seems that the ASSERT(list || value) has the same meaning as the ASSERT(value) in the line 1594. Other changes looks OK to me. >> Source/WebCore/css/CSSParser.cpp:1595 >> + if (value) { > > Why you assert here if you check with an if? Maybe because ASSERT() is ignored in the Release build. 599217 4 136389 menard 2012-04-10 06:33:54 -0700 Comment on attachment 136389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136389&action=review >>> Source/WebCore/css/CSSParser.cpp:1595 >>> + if (value) { >> >> Why you assert here if you check with an if? > > Maybe because ASSERT() is ignored in the Release build. I know it is but what's the point of asserting here in debug only if you protect the code with the if after, just avoiding the crash in Release but you may get erratic behavior. An example in WebKit code : bool AccessibilityRenderObject::isPasswordField() const { ASSERT(m_renderer); if (!m_renderer->node() || !m_renderer->node()->isHTMLElement()) ... m_renderer validity is not tested. To me assert means that if the assert is true well we're in bad shape, it should not happen. It's a nit pick maybe. 599223 5 136389 haraken 2012-04-10 06:41:14 -0700 Comment on attachment 136389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136389&action=review >>>> Source/WebCore/css/CSSParser.cpp:1595 >>>> + if (value) { >>> >>> Why you assert here if you check with an if? >> >> Maybe because ASSERT() is ignored in the Release build. > > I know it is but what's the point of asserting here in debug only if you protect the code with the if after, just avoiding the crash in Release but you may get erratic behavior. An example in WebKit code : > bool AccessibilityRenderObject::isPasswordField() const > { > ASSERT(m_renderer); > if (!m_renderer->node() || !m_renderer->node()->isHTMLElement()) > > ... > > m_renderer validity is not tested. > > To me assert means that if the assert is true well we're in bad shape, it should not happen. It's a nit pick maybe. Ah, makes sense. Maybe removing the if statement is a solution, since we are assuming that 'value' should not be 0 here. 599748 6 136389 macpherson 2012-04-10 15:52:29 -0700 Comment on attachment 136389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136389&action=review >> Source/WebCore/css/CSSParser.cpp:1581 >> + ASSERT(list || value); > > I am not sure what the ASSERT() is for. It seems that the ASSERT(list || value) has the same meaning as the ASSERT(value) in the line 1594. > > Other changes looks OK to me. This is to document (and enforce) the expected invariant that after the loop has executed, either value is set, or a list of values has been created. If neither of these are true, you have a problem. >>>>> Source/WebCore/css/CSSParser.cpp:1595 >>>>> + if (value) { >>>> >>>> Why you assert here if you check with an if? >>> >>> Maybe because ASSERT() is ignored in the Release build. >> >> I know it is but what's the point of asserting here in debug only if you protect the code with the if after, just avoiding the crash in Release but you may get erratic behavior. An example in WebKit code : >> bool AccessibilityRenderObject::isPasswordField() const >> { >> ASSERT(m_renderer); >> if (!m_renderer->node() || !m_renderer->node()->isHTMLElement()) >> >> ... >> >> m_renderer validity is not tested. >> >> To me assert means that if the assert is true well we're in bad shape, it should not happen. It's a nit pick maybe. > > Ah, makes sense. Maybe removing the if statement is a solution, since we are assuming that 'value' should not be 0 here. What's going on here is that static analysis tools look at the "while (value && value->unit == CSSPrimitiveValue::CSS_URI) {" above, and deduce that a possible exit condition of the loop is that value is null. It turns out that we expect that if that has happened we would have handled it in the if (list) case above. Again, this is non-obvious to a cursory reading, and too convoluted for coverity to pick up. So here, the ASSERT is to document the expectation that if value was null, it should have been handled above in the if (list) case, and the if (value) was added to reassure any static analysis tools that we really are not going to do a null-pointer dereference here. 600085 7 136389 haraken 2012-04-11 00:11:41 -0700 Comment on attachment 136389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=136389&action=review >>> Source/WebCore/css/CSSParser.cpp:1581 >>> + ASSERT(list || value); >> >> I am not sure what the ASSERT() is for. It seems that the ASSERT(list || value) has the same meaning as the ASSERT(value) in the line 1594. >> >> Other changes looks OK to me. > > This is to document (and enforce) the expected invariant that after the loop has executed, either value is set, or a list of values has been created. If neither of these are true, you have a problem. Think this way: Basically, ASSERT(... || ...) is not a good form since it is not intuitive. We want to write ASSERT(...) for one variable at the exact place. So let's consider how we can split ASSERT(list || value) into ASSERT(list) and ASSERT(value). Then the code will look like this: if (list) { if (!value) { ...; } } ASSERT(value); // already written if (value) { ...; } This means that the original ASSERT(list || value) is not needed. >>>>>> Source/WebCore/css/CSSParser.cpp:1595 >>>>>> + if (value) { >>>>> >>>>> Why you assert here if you check with an if? >>>> >>>> Maybe because ASSERT() is ignored in the Release build. >>> >>> I know it is but what's the point of asserting here in debug only if you protect the code with the if after, just avoiding the crash in Release but you may get erratic behavior. An example in WebKit code : >>> bool AccessibilityRenderObject::isPasswordField() const >>> { >>> ASSERT(m_renderer); >>> if (!m_renderer->node() || !m_renderer->node()->isHTMLElement()) >>> >>> ... >>> >>> m_renderer validity is not tested. >>> >>> To me assert means that if the assert is true well we're in bad shape, it should not happen. It's a nit pick maybe. >> >> Ah, makes sense. Maybe removing the if statement is a solution, since we are assuming that 'value' should not be 0 here. > > What's going on here is that static analysis tools look at the "while (value && value->unit == CSSPrimitiveValue::CSS_URI) {" above, and deduce that a possible exit condition of the loop is that value is null. It turns out that we expect that if that has happened we would have handled it in the if (list) case above. Again, this is non-obvious to a cursory reading, and too convoluted for coverity to pick up. So here, the ASSERT is to document the expectation that if value was null, it should have been handled above in the if (list) case, and the if (value) was added to reassure any static analysis tools that we really are not going to do a null-pointer dereference here. I think that WebKit normally does not write such "reassurance". abarth@, darin@ (who will know much about WebKit codebase): Any idea? 600709 8 macpherson 2012-04-11 16:42:35 -0700 Hi Kentaro, I still don't feel like we're on the same page here. This is hard to do in the context of a code review / bug tracker, but I think it would be helpful if you took a look at the original code, and try to explain why it is safe to dereference value on line 1593 "id = value->id;" I think once you go through that exercise it will become much clearer what this patch is trying to achieve. 600845 9 haraken 2012-04-11 20:39:01 -0700 (In reply to comment #8) > Hi Kentaro, I still don't feel like we're on the same page here. > > This is hard to do in the context of a code review / bug tracker, but I think it would be helpful if you took a look at the original code, and try to explain why it is safe to dereference value on line 1593 "id = value->id;" > > I think once you go through that exercise it will become much clearer what this patch is trying to achieve. As darktears@ pointed out, I think - If value->id should be always safe, put 'ASSERT(value)' and remove 'if(value)'. - If value->id can be unsafe, put 'if(value)' and remove 'ASSERT(value)'. The point is that having both 'ASSERT(value)' and 'if(value)' sounds strange. 604298 10 137593 macpherson 2012-04-17 13:39:59 -0700 Created attachment 137593 Patch 604306 11 137593 haraken 2012-04-17 13:46:39 -0700 Comment on attachment 137593 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=137593&action=review OK > Source/WebCore/css/CSSParser.cpp:1641 > + return false; Nit: What happens if we do not write 'return false'? Compile error? 604325 12 macpherson 2012-04-17 14:04:41 -0700 (In reply to comment #11) > (From update of attachment 137593 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=137593&action=review > > OK > > > Source/WebCore/css/CSSParser.cpp:1641 > > + return false; > > Nit: What happens if we do not write 'return false'? Compile error? The safest thing to do is to immediately stop treat the property as invalid if we did get to this state somehow. 604427 13 137593 webkit.review.bot 2012-04-17 15:56:40 -0700 Comment on attachment 137593 Patch Clearing flags on attachment: 137593 Committed r114455: <http://trac.webkit.org/changeset/114455> 604428 14 webkit.review.bot 2012-04-17 15:56:46 -0700 All reviewed patches have been landed. Closing bug. 136389 2012-04-09 20:59:31 -0700 2012-04-17 13:39:51 -0700 Patch bug-83544-20120410135929.patch text/plain 3145 macpherson U3VidmVyc2lvbiBSZXZpc2lvbjogMTEzNjY1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMDU3Y2IzYzgxZWJlOGQ2 OTI3NzJjZTk0MmNiMzU3YWQ1YWFjNjMxMS4uNTFjYmU5ZjM4ZTA3OGI3YjZmOGY2MDU5MDdiY2Ji Y2MwYjljYTkwMyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA0LTA5ICBMdWtl IE1hY3BoZXJzb24gIDxtYWNwaGVyc29uQGNocm9taXVtLm9yZz4KKworICAgICAgICBNYWtlIENT U1BhcnNlcjo6cGFyc2VWYWx1ZSgpJ3MgaGFuZGxpbmcgb2YgQ1NTUHJvcGVydHlDdXJzb3IgbW9y ZSBvYnZpb3VzbHkgY29ycmVjdC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTgzNTQ0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgTm8gbmV3IHRlc3RzIC8gY29kZSBjbGVhbnVwIG9ubHkuCisKKyAgICAgICAg VGhlIGNvZGUgYXMgaXQgc3RhbmRzIGFwcGVhcnMgdG8gYmUgY29ycmVjdCwgYnV0IHN0YXRpYyBh bmFseXNpcyB3YXMgY29uY2VybmVkIHRoYXQgdmFsdWUgY291bGQgYmVjb21lIG51bGwuCisgICAg ICAgIEkndmUgbWFkZSBzb21lIG9mIHRoZSBpbnZhcmlhbnRzIHRoYXQgaW50byBBU1NFUlRzIGFu ZCBhZGRlZCBhIG51bGwtY2hlY2sgdG8gbWFrZSBzdGF0aWMgYW5hbHlzaXMgZWFzaWVyLgorCisg ICAgICAgICogY3NzL0NTU1BhcnNlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpDU1NQYXJzZXI6 OnBhcnNlVmFsdWUpOgorCiAyMDEyLTA0LTA5ICBFbWlsIEEgRWtsdW5kICA8ZWFlQGNocm9taXVt Lm9yZz4KIAogICAgICAgICBSZXBsYWNlIG51bWVyaWNfbGltaXRzPExheW91dFVuaXQ+OjptaW4v bWF4IHdpdGggY29uc3RhbnRzCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9jc3MvQ1NTUGFy c2VyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuY3BwCmluZGV4IDEyNWU0NGEz NjI4NDIzOTBiYWUxMjAwMDU0MjAzYmZkNGMwNjZjNTMuLjc3MDUwNTc1ZDI0NDQ1ZjFjMmU2Njll MjczMGFkYWE5NmIyYWM5Y2UgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJz ZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuY3BwCkBAIC0xNTc4LDI0 ICsxNTc4LDI4IEBAIGJvb2wgQ1NTUGFyc2VyOjpwYXJzZVZhbHVlKENTU1Byb3BlcnR5SUQgcHJv cElkLCBib29sIGltcG9ydGFudCkKICAgICAgICAgICAgICAgICByZXR1cm4gZmFsc2U7CiAgICAg ICAgICAgICB2YWx1ZSA9IG1fdmFsdWVMaXN0LT5uZXh0KCk7IC8vIGNvbW1hCiAgICAgICAgIH0K KyAgICAgICAgQVNTRVJUKGxpc3QgfHwgdmFsdWUpOwogICAgICAgICBpZiAobGlzdCkgewogICAg ICAgICAgICAgaWYgKCF2YWx1ZSkgeyAvLyBubyB2YWx1ZSBhZnRlciB1cmwgbGlzdCAoTVNJRSA1 IGNvbXBhdGliaWxpdHkpCiAgICAgICAgICAgICAgICAgaWYgKGxpc3QtPmxlbmd0aCgpICE9IDEp CiAgICAgICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKICAgICAgICAgICAgIH0gZWxzZSBp ZiAoaW5RdWlya3NNb2RlKCkgJiYgdmFsdWUtPmlkID09IENTU1ZhbHVlSGFuZCkgLy8gTVNJRSA1 IGNvbXBhdGliaWxpdHkgOi8KICAgICAgICAgICAgICAgICBsaXN0LT5hcHBlbmQoY3NzVmFsdWVQ b29sKCkuY3JlYXRlSWRlbnRpZmllclZhbHVlKENTU1ZhbHVlUG9pbnRlcikpOwotICAgICAgICAg ICAgZWxzZSBpZiAodmFsdWUgJiYgKCh2YWx1ZS0+aWQgPj0gQ1NTVmFsdWVBdXRvICYmIHZhbHVl LT5pZCA8PSBDU1NWYWx1ZVdlYmtpdEdyYWJiaW5nKSB8fCB2YWx1ZS0+aWQgPT0gQ1NTVmFsdWVD b3B5IHx8IHZhbHVlLT5pZCA9PSBDU1NWYWx1ZU5vbmUpKQorICAgICAgICAgICAgZWxzZSBpZiAo KHZhbHVlLT5pZCA+PSBDU1NWYWx1ZUF1dG8gJiYgdmFsdWUtPmlkIDw9IENTU1ZhbHVlV2Via2l0 R3JhYmJpbmcpIHx8IHZhbHVlLT5pZCA9PSBDU1NWYWx1ZUNvcHkgfHwgdmFsdWUtPmlkID09IENT U1ZhbHVlTm9uZSkKICAgICAgICAgICAgICAgICBsaXN0LT5hcHBlbmQoY3NzVmFsdWVQb29sKCku Y3JlYXRlSWRlbnRpZmllclZhbHVlKHZhbHVlLT5pZCkpOwogICAgICAgICAgICAgbV92YWx1ZUxp c3QtPm5leHQoKTsKICAgICAgICAgICAgIHBhcnNlZFZhbHVlID0gbGlzdC5yZWxlYXNlKCk7CiAg ICAgICAgICAgICBicmVhazsKICAgICAgICAgfQotICAgICAgICBpZCA9IHZhbHVlLT5pZDsKLSAg ICAgICAgaWYgKGluUXVpcmtzTW9kZSgpICYmIHZhbHVlLT5pZCA9PSBDU1NWYWx1ZUhhbmQpIHsg Ly8gTVNJRSA1IGNvbXBhdGliaWxpdHkgOi8KLSAgICAgICAgICAgIGlkID0gQ1NTVmFsdWVQb2lu dGVyOwotICAgICAgICAgICAgdmFsaWRQcmltaXRpdmUgPSB0cnVlOwotICAgICAgICB9IGVsc2Ug aWYgKCh2YWx1ZS0+aWQgPj0gQ1NTVmFsdWVBdXRvICYmIHZhbHVlLT5pZCA8PSBDU1NWYWx1ZVdl YmtpdEdyYWJiaW5nKSB8fCB2YWx1ZS0+aWQgPT0gQ1NTVmFsdWVDb3B5IHx8IHZhbHVlLT5pZCA9 PSBDU1NWYWx1ZU5vbmUpCi0gICAgICAgICAgICB2YWxpZFByaW1pdGl2ZSA9IHRydWU7CisgICAg ICAgIEFTU0VSVCh2YWx1ZSk7CisgICAgICAgIGlmICh2YWx1ZSkgeworICAgICAgICAgICAgaWQg PSB2YWx1ZS0+aWQ7CisgICAgICAgICAgICBpZiAoaW5RdWlya3NNb2RlKCkgJiYgdmFsdWUtPmlk ID09IENTU1ZhbHVlSGFuZCkgeyAvLyBNU0lFIDUgY29tcGF0aWJpbGl0eSA6LworICAgICAgICAg ICAgICAgIGlkID0gQ1NTVmFsdWVQb2ludGVyOworICAgICAgICAgICAgICAgIHZhbGlkUHJpbWl0 aXZlID0gdHJ1ZTsKKyAgICAgICAgICAgIH0gZWxzZSBpZiAoKHZhbHVlLT5pZCA+PSBDU1NWYWx1 ZUF1dG8gJiYgdmFsdWUtPmlkIDw9IENTU1ZhbHVlV2Via2l0R3JhYmJpbmcpIHx8IHZhbHVlLT5p ZCA9PSBDU1NWYWx1ZUNvcHkgfHwgdmFsdWUtPmlkID09IENTU1ZhbHVlTm9uZSkKKyAgICAgICAg ICAgICAgICB2YWxpZFByaW1pdGl2ZSA9IHRydWU7CisgICAgICAgIH0KICAgICAgICAgYnJlYWs7 CiAgICAgfQogCg== 137593 2012-04-17 13:39:59 -0700 2012-04-17 15:56:40 -0700 Patch bug-83544-20120418063957.patch text/plain 2947 macpherson U3VidmVyc2lvbiBSZXZpc2lvbjogMTE0NDA1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNDk4NTVhYjQ2MGEwMmFm Mjg5NmM2NDRkZjA0NWJmYzM3MGIyNjY0OS4uNjgwOWU1NzY1NjE3NGI5NGVlNmU2MjRhYjdjYzVk Yjc0YWEyOTgxMCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDEyLTA0LTA5ICBMdWtl IE1hY3BoZXJzb24gIDxtYWNwaGVyc29uQGNocm9taXVtLm9yZz4KKworICAgICAgICBNYWtlIENT U1BhcnNlcjo6cGFyc2VWYWx1ZSgpJ3MgaGFuZGxpbmcgb2YgQ1NTUHJvcGVydHlDdXJzb3IgbW9y ZSBvYnZpb3VzbHkgY29ycmVjdC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hv d19idWcuY2dpP2lkPTgzNTQ0CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgTm8gbmV3IHRlc3RzIC8gY29kZSBjbGVhbnVwIG9ubHkuCisKKyAgICAgICAg VGhlIGNvZGUgYXMgaXQgc3RhbmRzIGFwcGVhcnMgdG8gYmUgY29ycmVjdCwgYnV0IHN0YXRpYyBh bmFseXNpcyB3YXMgY29uY2VybmVkIHRoYXQgdmFsdWUgY291bGQgYmVjb21lIG51bGwuCisgICAg ICAgIFRoaXMgcGF0Y2ggYWRkcyBhIG51bGwgY2hlY2sgYW5kIEFTU0VSVF9OT1RfUkVBQ0hFRCgp IHRvIG1ha2UgdGhlIGNvZGUgbW9yZSBvYnZpb3VzbHkgY29ycmVjdC4KKworICAgICAgICAqIGNz cy9DU1NQYXJzZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6Q1NTUGFyc2VyOjpwYXJzZVZhbHVl KToKKwogMjAxMi0wNC0xNyAgRW1pbCBBIEVrbHVuZCAgPGVhZUBjaHJvbWl1bS5vcmc+CiAKICAg ICAgICAgQWRkIHNpemVfdCB2ZXJzaW9ucyBvZiBtdWx0aXBsaWNhdGlvbiBhbmQgZGl2aXNpb24g b3BlcmF0b3JzIHRvIEZyYWN0aW9uYWxMYXlvdXRVbml0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi Q29yZS9jc3MvQ1NTUGFyc2VyLmNwcCBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIuY3Bw CmluZGV4IGI4M2JmZGRjZDNhOTJlY2UwNjNmNTMxNjgyMWM3ZGE3ZGI3ZTQ5ODYuLjk4YWJiMDE5 YjI2OWQ2OGY4NDA1Y2EzYzRkNmJhM2MxZDM0MTE5NGUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJD b3JlL2Nzcy9DU1NQYXJzZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2Nzcy9DU1NQYXJzZXIu Y3BwCkBAIC0xNjI0LDE4ICsxNjI0LDIyIEBAIGJvb2wgQ1NTUGFyc2VyOjpwYXJzZVZhbHVlKENT U1Byb3BlcnR5SUQgcHJvcElkLCBib29sIGltcG9ydGFudCkKICAgICAgICAgICAgICAgICAgICAg cmV0dXJuIGZhbHNlOwogICAgICAgICAgICAgfSBlbHNlIGlmIChpblF1aXJrc01vZGUoKSAmJiB2 YWx1ZS0+aWQgPT0gQ1NTVmFsdWVIYW5kKSAvLyBNU0lFIDUgY29tcGF0aWJpbGl0eSA6LwogICAg ICAgICAgICAgICAgIGxpc3QtPmFwcGVuZChjc3NWYWx1ZVBvb2woKS5jcmVhdGVJZGVudGlmaWVy VmFsdWUoQ1NTVmFsdWVQb2ludGVyKSk7Ci0gICAgICAgICAgICBlbHNlIGlmICh2YWx1ZSAmJiAo KHZhbHVlLT5pZCA+PSBDU1NWYWx1ZUF1dG8gJiYgdmFsdWUtPmlkIDw9IENTU1ZhbHVlV2Via2l0 R3JhYmJpbmcpIHx8IHZhbHVlLT5pZCA9PSBDU1NWYWx1ZUNvcHkgfHwgdmFsdWUtPmlkID09IENT U1ZhbHVlTm9uZSkpCisgICAgICAgICAgICBlbHNlIGlmICgodmFsdWUtPmlkID49IENTU1ZhbHVl QXV0byAmJiB2YWx1ZS0+aWQgPD0gQ1NTVmFsdWVXZWJraXRHcmFiYmluZykgfHwgdmFsdWUtPmlk ID09IENTU1ZhbHVlQ29weSB8fCB2YWx1ZS0+aWQgPT0gQ1NTVmFsdWVOb25lKQogICAgICAgICAg ICAgICAgIGxpc3QtPmFwcGVuZChjc3NWYWx1ZVBvb2woKS5jcmVhdGVJZGVudGlmaWVyVmFsdWUo dmFsdWUtPmlkKSk7CiAgICAgICAgICAgICBtX3ZhbHVlTGlzdC0+bmV4dCgpOwogICAgICAgICAg ICAgcGFyc2VkVmFsdWUgPSBsaXN0LnJlbGVhc2UoKTsKICAgICAgICAgICAgIGJyZWFrOworICAg ICAgICB9IGVsc2UgaWYgKHZhbHVlKSB7CisgICAgICAgICAgICBpZCA9IHZhbHVlLT5pZDsKKyAg ICAgICAgICAgIGlmIChpblF1aXJrc01vZGUoKSAmJiB2YWx1ZS0+aWQgPT0gQ1NTVmFsdWVIYW5k KSB7IC8vIE1TSUUgNSBjb21wYXRpYmlsaXR5IDovCisgICAgICAgICAgICAgICAgaWQgPSBDU1NW YWx1ZVBvaW50ZXI7CisgICAgICAgICAgICAgICAgdmFsaWRQcmltaXRpdmUgPSB0cnVlOworICAg ICAgICAgICAgfSBlbHNlIGlmICgodmFsdWUtPmlkID49IENTU1ZhbHVlQXV0byAmJiB2YWx1ZS0+ aWQgPD0gQ1NTVmFsdWVXZWJraXRHcmFiYmluZykgfHwgdmFsdWUtPmlkID09IENTU1ZhbHVlQ29w eSB8fCB2YWx1ZS0+aWQgPT0gQ1NTVmFsdWVOb25lKQorICAgICAgICAgICAgICAgIHZhbGlkUHJp bWl0aXZlID0gdHJ1ZTsKKyAgICAgICAgfSBlbHNlIHsKKyAgICAgICAgICAgIEFTU0VSVF9OT1Rf UkVBQ0hFRCgpOworICAgICAgICAgICAgcmV0dXJuIGZhbHNlOwogICAgICAgICB9Ci0gICAgICAg IGlkID0gdmFsdWUtPmlkOwotICAgICAgICBpZiAoaW5RdWlya3NNb2RlKCkgJiYgdmFsdWUtPmlk ID09IENTU1ZhbHVlSGFuZCkgeyAvLyBNU0lFIDUgY29tcGF0aWJpbGl0eSA6LwotICAgICAgICAg ICAgaWQgPSBDU1NWYWx1ZVBvaW50ZXI7Ci0gICAgICAgICAgICB2YWxpZFByaW1pdGl2ZSA9IHRy dWU7Ci0gICAgICAgIH0gZWxzZSBpZiAoKHZhbHVlLT5pZCA+PSBDU1NWYWx1ZUF1dG8gJiYgdmFs dWUtPmlkIDw9IENTU1ZhbHVlV2Via2l0R3JhYmJpbmcpIHx8IHZhbHVlLT5pZCA9PSBDU1NWYWx1 ZUNvcHkgfHwgdmFsdWUtPmlkID09IENTU1ZhbHVlTm9uZSkKLSAgICAgICAgICAgIHZhbGlkUHJp bWl0aXZlID0gdHJ1ZTsKICAgICAgICAgYnJlYWs7CiAgICAgfQogCg==