Closed Bug 466790 Opened 11 years ago Closed 11 years ago
file io is broken on windows ce
in win95io.c, we pass narrow char arguments to wide char functions with no conversion in several places.
Doug, can you take a look at this?
Assignee: wtc → blassey
Attachment #350137 - Flags: review?(doug.turner)
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.
Attachment #350137 - Flags: review?(doug.turner) → review-
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.
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.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.