327448 - Redundant implicit calls in NSPR

Status: RESOLVED FIXED

(NSPR :: NSPR, defect, P2)

Description

[Masatoshi Kimura [:emk]]

CreateFileW, FindFirstFileW, FindNextFileW, GetDriveTypeW, and GetFullPathNameW are  exported from Win95 kernel as stubs.
Therefore it's meaningless to call them implicitly.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: patch

I have confirmed this works on Win95.

Comment 2

[Wan-Teh Chang]

Comment on attachment 212102 [details] [diff] [review]
patch

Masatoshi: thank you for the patch.  If we
call the W functions on Windows 95, do they
fail with the ERROR_CALL_NOT_IMPLEMENTED error?

Do these W function stubs exist in Windows
95 even if you don't install the Microsoft
Layer for Unicode (MSLU)?

Comment 3

[Masatoshi Kimura [:emk]]

> If we call the W functions on Windows 95, do they
> fail with the ERROR_CALL_NOT_IMPLEMENTED error?
Yes.

> Do these W function stubs exist in Windows
> 95 even if you don't install the Microsoft
> Layer for Unicode (MSLU)?
Yes. I've confirmed it on very early version of Win95a without IE, without MSLU.
In general, Win95 Kernel32 exports all documented WinNT3.5 APIs (including all W functions) even if they are not implemented.

FYI W functions will succeed instead of failing with the ERROR_CALL_NOT_IMPLEMENTED error if we install MSLU.

Comment 4

[Masatoshi Kimura [:emk]]

> if we install MSLU.
if we install MSLU and our program link with unicows.lib.

Comment 5

[Wan-Teh Chang]

I checked in the patch on the NSPR trunk (NSPR 4.7).

Checking in w95io.c;
/cvsroot/mozilla/nsprpub/pr/src/md/windows/w95io.c,v  <--  w95io.c
new revision: 3.27; previous revision: 3.26
done

jshin, I'd still appreciate your review of the patch.

The Mozilla trunk is closed right now.  I will check
in the patch on the NSPRPUB_PRE_4_2_CLIENT_BRANCH when
the Mozilla trunk re-opens.

Comment 6

[Jungshik Shin]

Comment on attachment 212102 [details] [diff] [review]
patch

add. r=me (because I'm not a superreviewer). btw, these function pointers need to be included back later when we add UTF16 APIs on Win 9x/ME without MSLU (i.e. by emulating 'W' APIs ourselves).

Comment 7

[Wan-Teh Chang]

Attachment: Alternate patch: keep function pointers but do not lookup the W functions

Given jshin's comment 6, I think we should keep the
function pointers.  But we don't need to look up the
function symbols at run time.  Here is my proposed
alternate patch that reduces the amount of work when
we emulate the MSLU W functions.

jshin: in NSPR I'm using the superreview to mean
"second review".  Feel free to mark review+ and
cancel the super-review on patches you like.

Comment 8

[Masatoshi Kimura [:emk]]

Comment on attachment 212263 [details] [diff] [review]
Alternate patch: keep function pointers but do not lookup the W functions

It's better to initialize function pointers with W functions than to initialize them with NULL and assign W functions later. Because we are trying to eliminate NULL checks.
Leave InitUnicodeSupport() as an empty function for when we emulate the MSLU W functions.

BTW, these functions had not returned PR_NOT_IMPLEMENTED_ERROR because GetProcAddress() would never fail.
Perhaps it's better to add ERROR_CALL_NOT_IMPLEMENTED case in _MD_win32_map_default_error.

Comment 9

[Masatoshi Kimura [:emk]]

Attachment: Alternative patch, take 3.

Comment 10

[Wan-Teh Chang]

Comment on attachment 212270 [details] [diff] [review]
Alternative patch, take 3.

r=wtc. Thank you, Masatoshi.

Comment 11

[Jungshik Shin]

Comment on attachment 212270 [details] [diff] [review]
Alternative patch, take 3.

thanks. r=jshin

Comment 12

[Wan-Teh Chang]

I checked in the "alternative patch, take 3" on the NSPR
trunk (NSPR 4.7) and the NSPRPUB_PRE_4_2_CLIENT_BRANCH
(mozilla 1.9 alpha).

Checking in w95io.c;
/cvsroot/mozilla/nsprpub/pr/src/md/windows/w95io.c,v  <--  w95io.c
new revision: 3.29; previous revision: 3.28
done
Checking in win32_errors.c;
/cvsroot/mozilla/nsprpub/pr/src/md/windows/win32_errors.c,v  <--  win32_errors.c

new revision: 3.11; previous revision: 3.10
done

Checking in w95io.c;
/cvsroot/mozilla/nsprpub/pr/src/md/windows/w95io.c,v  <--  w95io.c
new revision: 3.22.4.6; previous revision: 3.22.4.5
done
Checking in win32_errors.c;
/cvsroot/mozilla/nsprpub/pr/src/md/windows/win32_errors.c,v  <--  win32_errors.c

new revision: 3.9.4.2; previous revision: 3.9.4.1
done

Comment 13

[Jungshik Shin]

The patch for bug 326168 got rejected in w95io.c (in NSPR corresponding to 1.8.x branch) because this bug hasn't been fixed in 1.8.x branch. Although it's trivial to modify it so that it can be applied to the branch,  it might be a good idea to fix this bug in 1.8.x branch as well.

Comment 14

[Darin Fisher]

Comment on attachment 212270 [details] [diff] [review]
Alternative patch, take 3.

a=darin for the MOZILLA_1_8_BRANCH

Comment 15

[Wan-Teh Chang]

Darin, if you check this patch in on the MOZILLA_1_8_BRANCH,
you also need to check it in on the NSPR_4_6_BRANCH and change
the target milestone to 4.6.2.

Comment 16

[Darin Fisher]

OK, good to know.  I'm not sure that we should check this in until we know that we wish to take the whole set of i18n changes on the MOZILLA_1_8_BRANCH.  Jshin: Can you set bug dependencies appropriately?

Comment 17

[Wan-Teh Chang]

I checked in the patch on the NSPR_4_6_BRANCH (NSPR 4.6.2)
and MOZILLA_1_8_BRANCH (Mozilla 1.8.1), to make it easier
to apply the patch for bug 326168.