466790 - file io is broken on windows ce

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

in win95io.c, we pass narrow char arguments to wide char functions with no conversion in several places.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: eliminates all the compiler warnings

Doug, can you take a look at this?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 350137 [details] [diff] [review]
eliminates all the compiler warnings

Preliminary remarks:
1) we have an 80 character line length limit for NSPR source code.  While 
there are some exceptions to this, (mistakes have been made :) we ask that 
all new code and changes respect that limit.

2) It REALLY HELPS for patches to be in CVS diff format, rather than this 
Hg git format.  Bugzilla just doesn't know how to handle the git format.
For one thing, it doesn't recognize that names such as 
a/nsprpub/pr/src/md/windows/w95io.c are even names of files in the tree, 
so it cannot show diffs in greater context, Witness
https://bugzilla.mozilla.org/attachment.cgi?action=diff&id=350137&collapsed=&headers=1&context=32

Consequently, reviewing these git format diffs is a LOT more work 
than it needs to be.  I believe it is more work to convert a GIT diff
to a CVS-style diff (something I've been doing repeatedly for these 
diffs) than to just produce the CVS style diff in the first place.   
So, I'm going to insist: going forward, NSS and NSPR patches must be 
in CVS patch format.  

About this patch:

This patch may work on WinCE but won't on desktops.
This patch assumes that _UNICODE is defined and that all TCHARS
are wide chars.  On WinCE, that is true by default.  Indeed, 
WinCE does not provide the "ANSI" (A suffix) versions of most 
functions that use TCHARS (or, it did not when I worked on WinCE
in 2002), so _UNICODE MUST be defined to build and run on WinCE.  

But on desktops, _UNICODE is NOT defined by default, and all the
macros that expand to the W suffix when it IS defined, but expand
to the A suffix when it is not, all expand to the A suffix by default.
On the desktop, FindFirstFile is a narrow "ANSI" function by default.

So, some of the changes made here, such as the change to the code 
that calls FindFirstFile, that assume that is always uses wide 
characters, are wrong on desktops.  (that's just one example)

I suggest you ifdef your patch, so that when _UNICODE is defined,
it does as your new code does and converts narrow to wide (and
vice versa) as needed, but when _UNICODE is NOT defined, it does
not, but uses the narrow strings without conversion.  _UNICODE
is exactly the right symbol to use for the #ifdef.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Having now read the companion patch for NSS, I see that you know about
these issues, so I gather that this patch's unconditional use of wide 
character strings with FindFirstFile (and elsewhere?) was merely an oversight.

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

That was a WIP patch that I just wanted to get doug's thoughts on, although I didn't really indicate that in any way.  When I have something worth checking in I'll ask for wtc's review.