First off, I think the high order bit is that if a non-null array reference is returned there are NO null String entries in it. Whether a zero length or null array is returned when there are no printers
is the less important issue.

However an empty array also tells you there are no printers, and you are less likely to get an NPE from that. It is arguably easier for the caller, if they don't need the extra null check first. FWIW the javax.print public APIs return zero length arrays instead of null and applications seem to survive :-)

I don't know what you mean by :
> (And anyway we need to handle the first null string reference)?

If you are referring to a small matter of coding in the native layer, then that is not an insurmountable problem.

Basically if there are problems getting names or whatever in the native layer, handle it THERE, don't make
everyone else have to deal with it.

One caveat: JNI calls can theoretically fail due to OOME .. in such a case we are doomed and the main goal is to not crash in native and to obey all JNI rules. What is returned in that case can be a null array reference and an NPE in a Java-level user of that reference in such a case is not a big deal.

-phil

On 12/6/18, 8:10 PM, Shashidhara Veerabhadraiah wrote:

Hi Phil, Is it advisable to return zero length array from native? The null return from native is telling the caller that there are no printers connected to the system. To tell this should we allocate a zero length array containing null back to the caller(And anyway we need to handle the first null string reference)? Handling the null on the caller isn't an easier option?

Thanks and regards,

Shashi

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

But what I am saying is we should not return a NULL string reference
from native. You are still allowing that and then having to handle
it in Java code.

And FWIW you can return a zero length array as well so there
isn't a NULL array reference to deal with either.

-phil.

On 12/3/18 8:29 AM, Shashidhara Veerabhadraiah wrote:

    Hi Phil, There were 2 problems earlier. One is that, from the
    native it is possible to have no printers being associated with
    the system(causing to return null reference) and other problem in
    the implementation was that there may be no network printers and
    still return a valid array reference containing a null string. The
    first problem is fixed by handling null references returned from
    this function and other problem was that earlier there were
    different base conditions, for updating the reference and use it
    to create the printer name array which could produce a valid
    reference pointing to null string. Here is the updated Webrev
    which fixes these problems:

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

    The big problem was that I was not able to reproduce this problem
    neither at my end nor at the systems used for PIT testing. Only
    Sergey had produced this NPE till now.

    Thanks and regards,

    Shashi

    *From:*Phil Race
    *Sent:* Wednesday, November 28, 2018 11:19 PM
    *To:* Shashidhara Veerabhadraiah
    <shashidhara.veerabhadra...@oracle.com>
    <mailto:shashidhara.veerabhadra...@oracle.com>; Prasanta Sadhukhan
    <prasanta.sadhuk...@oracle.com>
    <mailto:prasanta.sadhuk...@oracle.com>; 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

    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/
        <http://cr.openjdk.java.net/%7Esveerabhadra/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>
        <mailto:prasanta.sadhuk...@oracle.com>; Shashidhara
        Veerabhadraiah <shashidhara.veerabhadra...@oracle.com>
        <mailto:shashidhara.veerabhadra...@oracle.com>;
        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

        [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