Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

darwin: fix unnecessary include headers #795

Closed

Conversation

typester
Copy link
Contributor

@typester typester commented May 2, 2013

This file doesn't use any Cocoa functions, CoreFoundation.h is enough here.
This line causes compilation error on iOS environment.

And also, this file is compiled as Objective-C file but I think this is not necessary too.
This file doesn't use any Objective-C feature, and can be compiled as plain C file.
(This pull request doesn't care of this though)

This file doesn't use any Cocoa functions, CoreFoundation.h is enough here.
This line causes compilation error on iOS environment.
@bnoordhuis
Copy link
Contributor

Thanks, landed in 4b0fac8. I renamed the file to darwin-getproctitle.c in 9b801d5.

@bnoordhuis bnoordhuis closed this May 2, 2013
@lhecker
Copy link
Contributor

lhecker commented May 13, 2013

@bnoordhuis There is also no ApplicationServices/ApplicationServices.h and GetCurrentProcess() on the iOS platform. Wouldn't it be possible to just to use TargetConditionals.h and TARGET_OS_MAC to safely "disable" the uv__set_process_title() function on iOS?

@lhecker
Copy link
Contributor

lhecker commented May 13, 2013

Something like that:

diff --git a/src/unix/darwin-proctitle.c b/src/unix/darwin-proctitle.c
index e6c8590..7acb37b 100644
--- a/src/unix/darwin-proctitle.c
+++ b/src/unix/darwin-proctitle.c
@@ -19,8 +19,17 @@
  */

 #include <CoreFoundation/CoreFoundation.h>
-#include <ApplicationServices/ApplicationServices.h>

+#if defined(__APPLE__)
+# include <TargetConditionals.h>
+# if TARGET_OS_IPHONE
+
+int uv__set_process_title(const char* title) {
+  return -1;
+}
+
+# else
+#   include <ApplicationServices/ApplicationServices.h>

 int uv__set_process_title(const char* title) {
   typedef CFTypeRef (*LSGetCurrentApplicationASNType)(void);
@@ -77,3 +86,6 @@ int uv__set_process_title(const char* title) {

   return (err == noErr) ? 0 : -1;
 }
+
+# endif
+#endif

@bnoordhuis
Copy link
Contributor

Landed a slightly different take in f22163c.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants