I am not understanding you. I thought the problem to be we got an array of (say) 3 values (ie printer names) returned from native where some or all of the *values* were NULL. And I am saying we should in such a case in the native code, before returning,
remove from the returned array all values that are NULL.
That could result in an empty (zero length) array being returned from native but
no null names ..

-phil.

On 11/26/18 10:23 PM, Shashidhara Veerabhadraiah wrote:

The windows function EnumPrinters() returns a value that could be zero or greater. If it is zero we have no other option but to return null from the function. I do not think there is a way where we can always make sure we get a non-zero value considering the various system scenarios. So we should handle the null return values as well in the caller of this function I think.

Here is the updated Webrev for test fix:

http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.01/

Thanks and regards,
Shashi

*From:*Phil Race
*Sent:* Tuesday, November 27, 2018 1:52 AM
*To:* Prasanta Sadhukhan <prasanta.sadhuk...@oracle.com>; Shashidhara Veerabhadraiah <shashidhara.veerabhadra...@oracle.com>; 2d-dev@openjdk.java.net *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the print tests after JDK-8153732

[Removed swing-dev as this as nothing to do with swing].

You mention in the bug eval that you don't need a new test because this
is already covered by the test for 8153732. If that is the case then this
bugid should be added to that test.
Although it also looks like there are plenty of tests that provoke this ..
if all you need is a system without any printers then this is a serious regression.

I am not sure I am following why doCompare() is the place to fix this.
If getRemotePrinterNames() is returning NULL strings, then maybe it should not !

I suggest to fix it there.

-phil.


On 11/26/18 7:51 AM, Prasanta Sadhukhan wrote:

    I am not against doCompare() changes. I am just saying run()
    changes are not needed.

    Regards
    Prasanta

    On 26-Nov-18 9:15 PM, Shashidhara Veerabhadraiah wrote:

        There is a possible case and that is the reason for this fix.
        The possible states for prevRemotePinters and
        currentRemotePrinters are null and valid values and they can
        reach those states at any time either because of network
        disconnect or any other OS related changes. With that in mind,
        this fix tries to eliminate the possible NPE cases.

        Thanks and regards,

        Shashi

        *From:*Prasanta Sadhukhan
        *Sent:* Monday, November 26, 2018 8:01 PM
        *To:* Shashidhara Veerabhadraiah
        <shashidhara.veerabhadra...@oracle.com>
        <mailto:shashidhara.veerabhadra...@oracle.com>;
        swing-...@openjdk.java.net
        <mailto:swing-...@openjdk.java.net>; 2d-dev@openjdk.java.net
        <mailto:2d-dev@openjdk.java.net>
        *Subject:* Re: [OpenJDK 2D-Dev] [12] JDK-8212202: NPE in the
        print tests after JDK-8153732

        On 26-Nov-18 6:51 PM, shashidhara.veerabhadra...@oracle.com
        <mailto:shashidhara.veerabhadra...@oracle.com> wrote:

            Hi Prasanta, I think we should not create a behavior
            across the functions. doCompare() does only the comparison
            and it may be used for other purposes and is complete with
            respect to the comparison functionality.

            run() function has a different behavior as it needs to
            populate the prevRemotePrinters and then the
            currentRemotePrinters and then use the comparison
            functionality. I think this is a good way to do.

        Even without the if-else check, it does populates the
        prevRemotePrinters, no?
        if "prevRemotePrinters" is null and currentRemotePrinters is
        null, then no need to update. If currentRemotePrinters is
        null, then what is the point of using getRemotePrintersNames()
        to update prevRemotePrinters as currentRemotePrinters is also
        obtained from getRemotePrintersNames!!
        If "prevRemotePrinters" is null and currentRemotePrinters is
        not null, then doCompare() called from run() will be true and
        prevRemotePrinters will be populated with currentRemotePrinters.
        If "prevRemotePrinters" is not null and currentRemotePrinters
        is null, then also prevRemotePrinters will be populated with
        currentRemotePrinters.

        Regards
        Prasanta


            Thanks and regards,

            Shashi

            On 26/11/18 6:03 PM, Prasanta Sadhukhan wrote:

                Hi Shashi,

                I think l437 check of if-else *if (prevRemotePrinters
                != null) {*

                is not required. prevRemotePrinters null check is
                addressed in str1==null case in doCompare().
                If prevRemotePrinters is null and
                currentRemotePrinters is not null, then you update
                prevRemotePrinters to currentRemotePrinters as per
                l415 where doCompare returns true.
                Also, If prevRemotePrinters is not null and
                currentRemotePrinters is null, then also you update
                prevRemotePrinters to currentRemotePrinters which is
                the output of getRemotePrintersNames().

                Regards
                Prasanta


                On 26-Nov-18 2:33 PM, Shashidhara Veerabhadraiah wrote:

                    Hi All, Please review a NPE fix for the below bug.

                    Bug: https://bugs.openjdk.java.net/browse/JDK-8212202

                    Webrev:
                    http://cr.openjdk.java.net/~sveerabhadra/8212202/webrev.00/
                    
<http://cr.openjdk.java.net/%7Esveerabhadra/8212202/webrev.00/>

                    Function getRemotePrintersNames() may return null
                    values and hence they need to be handled from the
                    caller of that function which was missing earlier.
                    This fix handles the null return values of the
                    said function.

                    Thanks and regards,

                    Shashi


Reply via email to