RFR - JDK-8133416: [TESTBUG] Remove @ignore for closed/runtime/4345157/Prog.java

[Stas Smirnov]

Hi Dan,

thanks for your feedback, please see my comments below

On 06/11/15 22:07, Daniel D. Daugherty wrote:
> On 11/4/15 3:33 AM, Stas Smirnov wrote:
>> Hello,
>>
>> please review this fix for JDK-8133416.
>>
>> The test has been fixed and rewritten in Java to start using native 
>> support mechanism in Jtreg.
>> Also, its native part has been modified to avoid compilation on 
>> inappropriate platforms and also includes improvements.
>> Unfortunately the JBS issue is not visible to the community as well 
>> as the original version of the test sources.
>>
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8133416
>> webrev: http://cr.openjdk.java.net/~stsmirno/8133416/webrev.00
>
> make/test/JtregNative.gmk
>     No comments.
>
> test/runtime/ThreadSignalMask/Prog.java
>     No comments.
>
> test/runtime/ThreadSignalMask/ThreadSignalMask.java
>     No comments.
>
> test/runtime/ThreadSignalMask/exeThreadSignalMask.c
>     General: I think the indent standard for 'C' was also two spaces,
>         but I could be wrong...
I do not know about any strict requirement about spaces, just for me 4 
spaces are more readable, is it that critical?
>
>     L39 char path[PATH_MAX];
>         Recommend '+1' for NULL termination space.
>
>     L45: extern void exit(int);
>         Hard to believe this isn't in a header file...
warnings tell me that it is not
>
>     L50: fputs("Error occurred while closing handle", stderr);
>         Will this output include a newline?
thanks for catching this
>
>     L62:     char lib[PATH_MAX];
>         Recommend '+1' for NULL termination space.
>
>     L63: snprintf(lib, sizeof (lib), 
> "%s/lib/sparcv9/server/libjvm.so", path);
>         I don't think snprintf() will NULL terminate if the buffer is 
> full.
>         Add: lib[PATH_MAX] = '\0';
>
>     L180:     strncpy(path, argv[1], PATH_MAX);
>         strncpy() won't NULL terminate if buffer is full.
>         Add: path[PATH_MAX] = '\0';
actually from the manpage snprintf does include a NULL termination 
character and strncpy does not,
so I have wrote
strncpy(path, argv[1], PATH_MAX);
path[PATH_MAX - 1] = '\0';

and a question to you, since snprintf does handle NULL termination, will 
it be more reasonable to use it here as well,
snprintf(path, sizeof (path), "%s", argv[1] );
or, since we do not need any formatted string here just use two lines I 
wrote above?

If you are ok with it, I will not send a new webrev, just add this fixes 
in the result commit.
>
>
> I think that's it...
>
> Dan
>
>> Tested: the fix has been tested on all platforms with the 
>> hotspot/test/runtime testset
>