Dmitry,
src/share/demo/jvmti/hprof/hprof_init.c
L743-744: Indentation looks wrong
src/share/back/utf_util.h
L1: Missing copyright header.
L14: UTF_ERROR and UTF_ASSERT are only used in utf_util.c, so they can be
defined in that file instead of the header file.
src/share/back/utf_util.c
L297: Wouldn’t it make sense to cache the result of getCodepage() in a static
variable so we don’t need to call this code for each UTF conversion?
L492: skeep -> skip
L492: This TODO should be fixed. I also have the same worry here as for the
windows code: that we should cache data so that we don’t run this
unnecessarily.
In the original code we did:
/* Set the locale from the environment */
(void)setlocale(LC_ALL, "”);
but that is missing from the new code. I don’t know if it matters?
Thanks,
/Staffan
On 23 apr 2014, at 17:03, Dmitry Samersoff <dmitry.samers...@oracle.com> wrote:
> Hi Everybody,
>
> Please review the fix that removes npt library from jdk.
>
> CR link:
>
> https://bugs.openjdk.java.net/browse/JDK-8041498
>
> Webrev:
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8041498/webrev.01/
>
>
> Testing:
>
> nsk.jdwp
>
> I plan to submit a separate aurora run when I get all concerns.
>
> Fix details:
>
> * Today libnpt is a set of UTF related function and only few of them is
> actually used. The only users of npt is JDWP and hprof. So it's not
> necessary to keep it as a separate library.
>
> * Relevant code moved from npt to src/share/back/utf_util.c
> platfrom specific part is under #ifdef _WINDOWS initialization step
> is removed.
>
> * hprof changed to don't do any conversion.
>
> The only place where hrpof care about encoding is an output
> filename. Conversion from platform to UTF8 assume that
> filesystem does support UTF8 but command line doesn't.
> This assumption is obviously not correct in all cases and
> therefore handling of non-ascii filenames is incomplete.
> We should file a separate RFE if we need to handle no-ascii
> filenames by hprof.
>
> -Dmitry
>
> --
> Dmitry Samersoff
> Oracle Java development team, Saint Petersburg, Russia
> * I would love to change the world, but they won't give me the sources.
