Bug 100312 - Call ansi functions directly, to be able to compile with different -DUNICODE variants
Summary: Call ansi functions directly, to be able to compile with different -DUNICODE ...
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-03-22 07:00 UTC by Christoph Cullmann
Modified: 2017-08-16 11:44 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Patch to call A variants (3.30 KB, patch)
2017-03-22 07:00 UTC, Christoph Cullmann
Details | Splinter Review
Updated patch, missed some functions, compiles no in our CI (6.85 KB, patch)
2017-03-22 07:42 UTC, Christoph Cullmann
Details | Splinter Review

Description Christoph Cullmann 2017-03-22 07:00:42 UTC
Created attachment 130370 [details] [review]
Patch to call A variants

Instead of relying on the fact that we have no unicode on and the alias is the ansi function, call it explicitly.
Comment 1 Christoph Cullmann 2017-03-22 07:42:23 UTC
Created attachment 130371 [details] [review]
Updated patch, missed some functions, compiles no in our CI
Comment 2 Albert Astals Cid 2017-03-22 22:37:51 UTC
Tomas, Hib, you've sent some windows patches.

Comments on this?
Comment 3 Thomas Freitag 2017-03-23 09:08:02 UTC
The patch removes the ability of using unicode filesystem and force to use always the ANSI version of several functions. I.e. 

#ifdef UNICODE
#define GetModuleFileName  GetModuleFileNameW
#else
#define GetModuleFileName  GetModuleFileNameA
#endif // !UNICODE

is defined in winbase.h.
So I can't see why this patch makes sense?
Comment 4 Christoph Cullmann 2017-03-23 09:10:11 UTC
> So I can't see why this patch makes sense?
There is no way to use the unicode variants as all calling code assumes you can feed in char * everywhere.

If you want to use the unicode variants, you need to rewrite all code calling these things.

(Otherwise it would compile with UNICODE set, btw.)
Comment 5 Adrian Johnson 2017-03-23 10:41:50 UTC
(In reply to Christoph Cullmann from comment #1)
> Created attachment 130371 [details] [review] [review]
> Updated patch, missed some functions, compiles no in our CI

I agree with explicitly specifying the A or W variant for all win32 API that takes strings.

The win32 code in gfile.cc should be updated to accept UTF8 strings and convert to UTF16 and call the W variant to ensure full unicode support. This would avoid all the win32 specific code in other locations eg PDFDoc.cc line 173. We should consistently use UTF8 in poppler code and just do the conversion to wchar_t in the win32 portability code.
Comment 6 Christoph Cullmann 2017-04-21 16:12:49 UTC
> I agree with explicitly specifying the A or W variant for all win32 API that takes strings.

Does that mean this can be merged?
Comment 7 Albert Astals Cid 2017-04-25 20:08:13 UTC
(In reply to Christoph Cullmann from comment #6)
> > I agree with explicitly specifying the A or W variant for all win32 API that takes strings.
> 
> Does that mean this can be merged?

I don't know the second paragraph from Adrian goes into "i don't really care enough about poppler/Windows to try to understand that".

So i'll leave it into Adrian's hand since at least he seems to understand enough of it ;)
Comment 8 Adrian Johnson 2017-08-16 11:44:20 UTC
Pushed


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.