<AWT Dev>  RFR : JDK-8198000 : java/awt/List/EmptyListEventTest/EmptyListEventTest.java debug assert on Windows
philip.race at oracle.com
Sat Nov 3 17:28:46 UTC 2018
- I am not sure you can assume the LOWORD value is non-negative.
It seems to me that the 0XFFFF we got back is meant to be
interpreted as "-1"
which is what I wrote below.
I do note that it appears that in issue #2 you are seeing that up to
may be allowed and it wraps if you add more than 65535 ..
So *perhaps* we can interpret 0xFFFF as meaning positive 65535 but I
*only* if HIWORD is "0".
- Issue #1 : I am not sure that a "1" in HIWORD automatically means it
is off the
end of the list ... just "outside the client area". I think if HIWORD is
1 we just
bail don't we ?
Then you don't have to worry about whether 0xFFFF meant -1 or 65536
- Issue #2 : There is always *some* limit in cases like this. 32767
(2^15-1) or 65535 (2^16-1)
are very typical in these platform APIs. Often the platform doesn't
explicitly document it
and you have to infer it from the data type. I think it was all very
moot when these APIs
were designed because you'd run out of memory before you could get that
many items :-)
I'd be surprised if there were not already some open bug pointing out
that we accept "int"
for index and don't have any feedback when exceeding platform limits.
From a compatibility point of view, I don't think it is worth doing
anything that would
break ancient applications to provide that feedback.
On 11/2/18, 10:23 AM, Ambarish Rapte wrote:
> Hi Phil,
> Thanks for guiding with the documentation.
> The fix is modified after a relook at the documentation, and
> observed two issues [mentioned below].
> webrev: http://cr.openjdk.java.net/~arapte/8198000/webrev.02
> Fix Justification:
> 1. Message LB_ITEMFROMPOINT returns a LONG value.
> 2. There is no mention of LB_ERR as return value, like it is
> mentioned clearly for some of the other messages as LB_GETCOUNT,
> LB_SETITEMHEIGHT, LB_GETTEXT, LB_GETCURSEL.
> * So the existing comparison against LB_ERR is incorrect.
> 3. The two parts LOWORD and HIWORD of return value are of type
> WORD(unsigned short), so the return value can never be negative.
> * It is another reason to not to compare the returned index
> value with LB_ERR which is defined as (-1).
> Extract the index from LOWORD into a WORD variable and verify only
> if the index is smaller than the list size. (webrev.02)
> This fixes both JDK-8198000 & below mentioned Issue 1.
> 2 other Issues:
> Issue 1: NON selection of an item.
> 1. Currently the returned LONG value is used without extracting the
> 2. As far as the HIWORD is 0, the LONG return value would be same as
> index of item(LOWORD).
> 3. But when HIWORD is 1, the LONG return value would be a large
> unexpected value. If it is used as an index, then it would result
> in NON selection of the item.
> 4. Test program:
> 5. Steps:
> 1. Compile and execute the test program with release build JDK.
> 2. Click in the list's client area below the last item, i.e. do
> not click on any item.
> 6. Expected behavior: Last item should get selected.
> 7. Actual behavior: Last item does not get selected on first click.
> But the focused item gets selected after few clicks.
> Issue 2: Incorrect selection when list size exceeds sizeof(WORD)
> 1. Compile and execute the program with release build JDK (with or
> without the fix) :
> 2. Click the first item in list, press End key.
> 3. Click the last item 65544.
> 4. Expected behavior: The item 65544 should get selected.
> 5. Actual behavior: The item 65544 does not get selected and instead
> item 8 gets selected.
> 6. Verified only windows behavior.
> 7. I suggest to file a new JBS for this issue.
> *From:*Phil Race
> *Sent:* Thursday, November 01, 2018 1:51 AM
> *To:* Ambarish Rapte <ambarish.rapte at oracle.com>; awt-dev at openjdk.java.net
> *Subject:* Re: <AWT Dev>  RFR : JDK-8198000 :
> java/awt/List/EmptyListEventTest/EmptyListEventTest.java debug assert
> on Windows
> That adds what I suggested, but I had also suggested you leave in what you
> had added as it also adds some protection.
> Additionally I read the MS docs and they do explain the 131071 return
> The message this code is sending is LB_ITEMFROMPOINT and the docs say
> >The return value contains the index of the nearest item in the
> <https://msdn.microsoft.com/library/windows/desktop/ms632659>. The
> *HIWORD* <https://msdn.microsoft.com/library/windows/desktop/ms632657>
> is zero if the > specified point is in the client area of the list
> box, or one if it is outside the client area"
> You got 131071 which is, in hex 0X1FFFF.
> So you got "1" for hi-word, meaning "outside client area" and "-1" for
> meaning the index. And a return index of "-1" doubtless means an error ..
> On 10/31/18 12:49 PM, Ambarish Rapte wrote:
> Hi Phil & Sergey,
> This issue was introduced with the fix for JDK-6806217
> <https://bugs.openjdk.java.net/browse/JDK-6806217>, in 7b55, which
> modified AwtList::HandleEvent(), so It was not observed with JDK6.
> Please review the updated change as discussed
> offline: http://cr.openjdk.java.net/~arapte/8198000/webrev.01/
> *From:*Phil Race
> *Sent:* Tuesday, October 30, 2018 2:09 AM
> *To:* Ambarish Rapte <ambarish.rapte at oracle.com>
> <mailto:ambarish.rapte at oracle.com>; awt-dev at openjdk.java.net
> <mailto:awt-dev at openjdk.java.net>
> *Subject:* Re: <AWT Dev>  RFR : JDK-8198000 :
> java/awt/List/EmptyListEventTest/EmptyListEventTest.java debug
> assert on Windows
> On 10/29/18 7:31 AM, Ambarish Rapte wrote:
> Please review this Windows only fix,
> 1. Test asserts with debug build jdk, only on windows.
> Only the debug build turns on asserts.
> But I think JDK 6 always turned on asserts, and this test was
> introduced in 6,
> so something must have changed else we'd have seen this test fail
> a long time ago.
> Can you identify what it was ?
> Also same comment as the other bug - you need to add the bug id to
> the test.
> 3. Assert at Line no 77, awt_List.h :: IsItemSelected()
> 4. awt_List.cpp :: AwtList::HandleEvent() calls
> IsItemSelected() with an incorrect index value.
> Why ?
> 6. In AwtList::HandleEvent() , the call
> SendListMessage(LB_ITEMFROMPOINT, 0, msg->lParam) returns
> an arbitrary value 131071, which gets passed to
> 7. Could not find any relevance to the value 131071, from
> LB_ITEMFROMPOINT doc.
> That is (128*1024)-1, so it is probably not "arbitrary".
> Please add the eval above to the bug report .. once we have a
> complete understanding.
> Fix: Index should be verified before making call to
> IsItemSelected() :
> Verification: All list tests pass.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the awt-dev