Skip to content

CoreFoundation cleanup for Windows #1828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 18, 2019
Merged

CoreFoundation cleanup for Windows #1828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0ea450d
CoreFoundation: use modern standards for DLL build
compnerd Jan 17, 2019
8e36a45
CoreFoundation: define `_CFThreadIsMainThread` on Win32
compnerd Jan 17, 2019
cfd1434
CoreFoundation: remove `__CFWindowsNamedPipeInitialize`
compnerd Jan 17, 2019
f72877d
CoreFoundation: remove `CFCopySearchPathForDirectoriesInDomains` on W…
compnerd Jan 17, 2019
39d47ff
CoreFoundation: remove `_CFGetWindowsAppleSystemLibraryDirectory`
compnerd Jan 17, 2019
a3d0f7a
CoreFoundation: guard some declarations
compnerd Jan 17, 2019
137e558
CoreFoundation: provide locale aliases on Windows
compnerd Jan 17, 2019
c2a79a6
CoreFoundation: remove some UID interface on Win32
compnerd Jan 17, 2019
a497812
CoreFoundation: we only support /MD builds on Winddows
compnerd Jan 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,11 @@ ExternalProject_Add(CoreFoundation
${CMAKE_COMMAND}
CMAKE_ARGS
-DBUILD_SHARED_LIBS=NO
-DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
# NOTE(compnerd) ensure that we build it as a DLL as we
# wish to re-export the symbols from Foundation
-DCMAKE_C_FLAGS=-D_USRDLL
-DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
-DCMAKE_INSTALL_LIBDIR=usr/lib
Expand Down
20 changes: 12 additions & 8 deletions CoreFoundation/Base.subproj/CFBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,20 @@
#endif

#if TARGET_OS_WIN32
#if !defined(CF_EXPORT)
#if defined(CF_BUILDING_CF) && defined(__cplusplus)
#define CF_EXPORT extern "C" __declspec(dllexport)
#elif defined(CF_BUILDING_CF) && !defined(__cplusplus)
#define CF_EXPORT extern __declspec(dllexport)
#elif defined(__cplusplus)
#define CF_EXPORT extern "C" __declspec(dllimport)
#if defined(__cplusplus)
#define _CF_EXTERN extern "C"
#else
#define _CF_EXTERN extern
#endif

#if defined(_USRDLL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is defined by the Windows SDK or compiler when building shared libraries?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a slight abuse of the normal behaviour for Windows. _USRDLL is pretty commonly defined (by Visual Studio's default build rules), but it technically is meant for ATL/MFC applications building a COM server for a DLL (InProc server).

#if defined(CoreFoundation_EXPORTS)
#define CF_EXPORT _CF_EXTERN __declspec(dllexport)
#else
#define CF_EXPORT extern __declspec(dllimport)
#define CF_EXPORT _CF_EXTERN __declspec(dllimport)
#endif
#else
#define CF_EXPORT _CF_EXTERN
#endif
#else
#define CF_EXPORT extern
Expand Down
4 changes: 3 additions & 1 deletion CoreFoundation/Base.subproj/CFPlatform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,13 @@ const char *_CFProcessPath(void) {
}
#endif

#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_EMBEDDED_MINI
#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_EMBEDDED_MINI || DEPLOYMENT_TARGET_WINDOWS
CF_CROSS_PLATFORM_EXPORT Boolean _CFIsMainThread(void) {
return pthread_main_np() == 1;
}
#endif

#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_EMBEDDED_MINI
const char *_CFProcessPath(void) {
if (__CFProcessPath) return __CFProcessPath;
#if DEPLOYMENT_TARGET_MACOSX
Expand Down
7 changes: 5 additions & 2 deletions CoreFoundation/Base.subproj/CFPriv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,12 @@ CF_EXPORT void _CFRuntimeSetCFMPresent(void *a);
CF_EXPORT const char *_CFProcessPath(void);
CF_EXPORT const char **_CFGetProcessPath(void);
CF_EXPORT const char **_CFGetProgname(void);

#if !TARGET_OS_WIN32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We like to keep these in the positive sense if possible

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, would you prefer:

#if TARGET_OS_WIN32
#else
...
#endif

Trying to exhaustively enumerate the other targets seems like a failing exercise since most OSes are Unixy :-(. But, if that really is preferable, I can attempt to do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, maybe it's ok. I'm not sure if we are super consistent about it anyway.

CF_EXPORT void _CFGetUGIDs(uid_t *euid, gid_t *egid);
CF_EXPORT uid_t _CFGetEUID(void);
CF_EXPORT uid_t _CFGetEGID(void);

#endif

#if (TARGET_OS_MAC && !(TARGET_OS_EMBEDDED || TARGET_OS_IPHONE || TARGET_OS_LINUX))
CF_EXPORT void _CFRunLoopSetCurrent(CFRunLoopRef rl);
Expand Down Expand Up @@ -220,8 +222,10 @@ typedef CF_OPTIONS(CFOptionFlags, CFSearchPathDomainMask) {
kCFAllDomainsMask = 0x0ffff /* all domains: all of the above and more, future items */
};

#if TARGET_OS_MAC || TARGET_OS_EMBEDDED
CF_EXPORT
CFArrayRef CFCopySearchPathForDirectoriesInDomains(CFSearchPathDirectory directory, CFSearchPathDomainMask domainMask, Boolean expandTilde);
#endif


/* Obsolete keys */
Expand Down Expand Up @@ -609,7 +613,6 @@ CF_EXPORT CFPropertyListRef _CFBundleCreateFilteredLocalizedInfoPlist(CFBundleRe

CF_EXPORT CFStringRef _CFGetWindowsAppleAppDataDirectory(void);
CF_EXPORT CFArrayRef _CFGetWindowsBinaryDirectories(void);
CF_EXPORT CFStringRef _CFGetWindowsAppleSystemLibraryDirectory(void);

// If your Windows application does not use a CFRunLoop on the main thread (perhaps because it is reserved for handling UI events via Windows API), then call this function to make distributed notifications arrive using a different run loop.
CF_EXPORT void _CFNotificationCenterSetRunLoop(CFNotificationCenterRef nc, CFRunLoopRef rl);
Expand Down
5 changes: 0 additions & 5 deletions CoreFoundation/Base.subproj/CFRuntime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1193,11 +1193,6 @@ void __CFInitialize(void) {
CFNumberGetTypeID(); // NB: This does other work

__CFCharacterSetInitialize();

#if DEPLOYMENT_TARGET_WINDOWS
__CFWindowsNamedPipeInitialize();
#endif

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we remove this completely?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nearly completely ... had two straggling references remaining which I just cleaned up :-).

__CFDateInitialize();

#if DEPLOYMENT_RUNTIME_SWIFT
Expand Down
1 change: 0 additions & 1 deletion CoreFoundation/Base.subproj/CFRuntime_Internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ CF_PRIVATE void __CFCharacterSetInitialize(void);

#if TARGET_OS_WIN32
CF_PRIVATE void __CFTSDWindowsInitialize(void);
CF_PRIVATE void __CFWindowsNamedPipeInitialize(void);
#endif

#if TARGET_OS_MAC || TARGET_OS_IPHONE || TARGET_OS_WIN32
Expand Down
2 changes: 1 addition & 1 deletion CoreFoundation/Base.subproj/CFSystemDirectories.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ CFSearchPathEnumerationState __CFGetNextSearchPathEnumeration(CFSearchPathEnumer
#endif


#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_WINDOWS
#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED

CFArrayRef CFCopySearchPathForDirectoriesInDomains(CFSearchPathDirectory directory, CFSearchPathDomainMask domainMask, Boolean expandTilde) {
CFMutableArrayRef array;
Expand Down
2 changes: 2 additions & 0 deletions CoreFoundation/Base.subproj/ForFoundationOnly.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,7 @@ CF_EXPORT const void *_CFArrayCheckAndGetValueAtIndex(CFArrayRef array, CFIndex
CF_EXPORT void _CFArrayReplaceValues(CFMutableArrayRef array, CFRange range, const void *_Nullable * _Nullable newValues, CFIndex newCount);


#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED
/* Enumeration
Call CFStartSearchPathEnumeration() once, then call
CFGetNextSearchPathEnumeration() one or more times with the returned state.
Expand All @@ -475,6 +476,7 @@ CF_EXPORT void _CFArrayReplaceValues(CFMutableArrayRef array, CFRange range, con
typedef CFIndex CFSearchPathEnumerationState;
CF_EXPORT CFSearchPathEnumerationState __CFStartSearchPathEnumeration(CFSearchPathDirectory dir, CFSearchPathDomainMask domainMask);
CF_EXPORT CFSearchPathEnumerationState __CFGetNextSearchPathEnumeration(CFSearchPathEnumerationState state, UInt8 *path, CFIndex pathSize);
#endif

/* For use by NSNumber and CFNumber.
Hashing algorithm for CFNumber:
Expand Down
10 changes: 10 additions & 0 deletions CoreFoundation/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,9 +326,19 @@ elseif(CMAKE_SYSTEM_NAME STREQUAL Darwin)
PRIVATE
-DDEPLOYMENT_TARGET_MACOSX)
elseif(CMAKE_SYSTEM_NAME STREQUAL Windows)
# NOTE(compnerd) we only support building with the dynamic CRT as using the
# static CRT causes problems for users of the library.
target_compile_definitions(CoreFoundation
PRIVATE
-D_DLL)
target_compile_definitions(CoreFoundation
PRIVATE
-DDEPLOYMENT_TARGET_WINDOWS)
if(BUILD_SHARED_LIBS)
target_compile_definitions(CoreFoundation
PRIVATE
-D_USRDLL)
endif()
endif()
target_compile_definitions(CoreFoundation
PRIVATE
Expand Down
2 changes: 1 addition & 1 deletion CoreFoundation/Error.subproj/CFError.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ static CFTypeRef _CFErrorPOSIXCallBack(CFErrorRef err, CFStringRef key) CF_RETUR
if (!errStr) return NULL;
if (CFEqual(key, kCFErrorDescriptionKey)) return errStr; // If all we wanted was the non-localized description, we're done

#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED || DEPLOYMENT_TARGET_WINDOWS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, why is this concept not portable to Windows?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows doesn't really have POSIX-y errors. We can translate errors using FormatMessage which will take care of the localization. However, Rather than trying to implement something for that, it seemed that _CFErrorPOSIXCallBack was invoked from one site, _CFErrorInitializeCallBackTable, which is used by CFError.*CallBackBlockForDomain which is currently not in use in Foundation ... so I thought I would take the lazy way out :-p.

#if DEPLOYMENT_TARGET_MACOSX || DEPLOYMENT_TARGET_EMBEDDED
// We need a kCFErrorLocalizedFailureReasonKey, so look up a possible localization for the error message
// Look for the bundle in /System/Library/CoreServices/CoreTypes.bundle
CFArrayRef paths = CFCopySearchPathForDirectoriesInDomains(kCFLibraryDirectory, kCFSystemDomainMask, false);
Expand Down
2 changes: 1 addition & 1 deletion CoreFoundation/Locale.subproj/CFLocaleKeys.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ CONST_STRING_DECL(kCFCalendarIdentifierEthiopicAmeteMihret, "ethiopic");
CONST_STRING_DECL(kCFCalendarIdentifierEthiopicAmeteAlem, "ethiopic-amete-alem");

// Aliases for Linux
#if DEPLOYMENT_TARGET_LINUX
#if DEPLOYMENT_TARGET_LINUX || DEPLOYMENT_TARGET_WINDOWS

CF_EXPORT CFStringRef const kCFBuddhistCalendar __attribute__((alias ("kCFCalendarIdentifierBuddhist")));
CF_EXPORT CFStringRef const kCFChineseCalendar __attribute__((alias ("kCFCalendarIdentifierChinese")));
Expand Down
6 changes: 1 addition & 5 deletions CoreFoundation/NumberDate.subproj/CFTimeZone.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,14 +679,10 @@ CFTimeZoneRef CFTimeZoneCreateWithWindowsName(CFAllocatorRef allocator, CFString
return retval;
}

extern CFStringRef _CFGetWindowsAppleSystemLibraryDirectory(void);
static void __InitTZStrings(void) {
static CFLock_t __CFTZDirLock = CFLockInit;
__CFLock(&__CFTZDirLock);
if (!__tzZoneInfo) {
CFStringRef winDir = _CFGetWindowsAppleSystemLibraryDirectory();
__tzZoneInfo = CFStringCreateWithFormat(NULL, NULL, CFSTR("%@\\etc\\zoneinfo"), winDir);
}
// TODO(compnerd) figure out how to initialize __tzZoneInfo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll need to make sure this works before merging right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, the interesting thing is that there is __CFCopyWindowsTimeZoneList which will enumerate the data from the registry. We reference that in CFTimeZoneCopyKnownNames. I don't have the context on why CoreFoundation attempted to provide its own copy of the timezone data rather than using the system one on Windows (as it already uses the system provided one on Darwin, BSD, and Linux).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think our Windows version of CF used to bring its own, but using the system TZ data is almost certainly better, since it can change underneath us.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, that's why I was okay with this being addressed subsequently when we can start playing with Foundation on Windows :-).

if (!__tzDir && __tzZoneInfo) {
int length = CFStringGetLength(__tzZoneInfo) + sizeof("\\zone.tab") + 1;
__tzDir = malloc(length); // If we don't use ascii, we'll need to malloc more space
Expand Down