From 09f15d2b8d6e6bfa8d532adaeda5c8b894394943 Mon Sep 17 00:00:00 2001
From: Richard Frith-MacDonald <rfm@gnu.org>
Date: Mon, 15 Feb 2016 10:10:39 +0000
Subject: [PATCH] Use defaults cleanups

git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@39373 72102866-910b-0410-8b05-ffd578937521
---
 ChangeLog                                 |  13 ++
 Headers/Foundation/NSUserDefaults.h       |  11 +-
 Headers/GNUstepBase/NSDebug+GNUstepBase.h |   2 +-
 Source/NSFileManager.m                    |  14 +-
 Source/NSPropertyList.m                   |   4 +-
 Source/NSUserDefaults.m                   | 213 ++++++++++++++--------
 6 files changed, 168 insertions(+), 89 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a1c3edece..86d259fa8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2016-02-15  Richard Frith-Macdonald <rfm@gnu.org>
+
+	* Source/NSFileManager.m: Use NSDebugLog for most debug warnings
+	* Source/NSUserDefaults.m: Restructure to cache flag information
+	from NSProcessInfo directly on initialization, so it can be used
+	when parsing property lists in the argument domain, avoiding any
+	recursion trying to parse arguments.  Remove obsolete code for
+	for parsing -GS and --GS argument prefixes specially.
+	Make sure that argument parsiong behavior matches OSX apart from
+	The handling of a lone '-' used as key/value, and for this case
+	document the difference (since the GNUstep behavior is more useful
+	and changing it would have a real chance of breaking existing code).
+
 2016-02-12  Niels Grewe <niels.grewe@halbordnung.de>
 
 	* Headers/Foundation/NSObjCRuntime.h: Define macros for
diff --git a/Headers/Foundation/NSUserDefaults.h b/Headers/Foundation/NSUserDefaults.h
index 7aa23c8fc..54b44af55 100644
--- a/Headers/Foundation/NSUserDefaults.h
+++ b/Headers/Foundation/NSUserDefaults.h
@@ -46,8 +46,15 @@ extern "C" {
 /* Standard domains */
 
 /**
- *  User defaults domain for process arguments.  Command-line arguments
- *  (attribute-value pairs, as in "-NSFoo bar") are placed into this domain.
+ * User defaults domain for process arguments.  Command-line arguments
+ * (key-value pairs, as in "-NSFoo bar") are placed in this domain.<br />
+ * Where there is a sequence of arguments beginning with '-', only the
+ * last one is used (so "-a -b -c d" will produce a single user default
+ * 'c' with value 'd').<br />
+ * NB. On OSX the argument "-" means a key consisting of an empty string
+ * (so you can't use a '-' as a default value), while in GNUstep a "-" is
+ * a special case which does not mean a default key (so '-' may be used
+ * as a value).<br />
  */
 GS_EXPORT NSString* const NSArgumentDomain;
 
diff --git a/Headers/GNUstepBase/NSDebug+GNUstepBase.h b/Headers/GNUstepBase/NSDebug+GNUstepBase.h
index dc22cfcee..9050e994a 100644
--- a/Headers/GNUstepBase/NSDebug+GNUstepBase.h
+++ b/Headers/GNUstepBase/NSDebug+GNUstepBase.h
@@ -303,7 +303,7 @@ GS_EXPORT NSString*	GSDebugMethodMsg(id obj, SEL sel, const char *file,
 
 /** The DLog macro is a less powerful but commonly used logging macro,
  * defined here for convenience when porting code.  It will tell you
- * the function name and line number but not the fle location.
+ * the function name and line number but not the file location.
  * It performs unconditional logging but is only compiled in when the
  * program is built with DEBUG defined.
  */
diff --git a/Source/NSFileManager.m b/Source/NSFileManager.m
index 89bc00c51..fbceecb70 100644
--- a/Source/NSFileManager.m
+++ b/Source/NSFileManager.m
@@ -1018,7 +1018,7 @@ static NSStringEncoding	defaultEncoding;
 	    NSFileOwnerAccountName, NSUserName(), nil];
 	  if (![self changeFileAttributes: attributes atPath: path])
 	    {
-	      NSLog(@"Failed to change ownership of '%@' to '%@'",
+	      NSDebugLog(@"Failed to change ownership of '%@' to '%@'",
 		path, NSUserName());
 	    }
 	}
@@ -2105,7 +2105,7 @@ static NSStringEncoding	defaultEncoding;
 
   return [NSDictionary dictionaryWithObjects: values forKeys: keys count: 5];
 #else
-  NSLog(@"NSFileManager", @"no support for filesystem attributes");
+  GSOnceMLog(@"NSFileManager", @"no support for filesystem attributes");
   ASSIGN(_lastError, @"no support for filesystem attributes");
   return nil;
 #endif
@@ -2389,7 +2389,7 @@ static inline void gsedRelease(GSEnumeratedDirectory X)
         }
       else
         {
-          NSLog(@"Failed to recurse into directory '%@' - %@", path,
+          NSDebugLog(@"Failed to recurse into directory '%@' - %@", path,
             [NSError _last]);
         }
     }
@@ -2540,7 +2540,7 @@ static inline void gsedRelease(GSEnumeratedDirectory X)
 		}
 	      if (S_IFDIR == (S_IFMT & statbuf.st_mode))
 		{
-		  _DIR*  dir_pointer;
+		  _DIR  *dir_pointer;
 
 		  dir_pointer
 		    = _OPENDIR([_mgr fileSystemRepresentationWithPath:
@@ -2556,7 +2556,7 @@ static inline void gsedRelease(GSEnumeratedDirectory X)
 		    }
 		  else
 		    {
-		      NSLog(@"Failed to recurse into directory '%@' - %@",
+		      NSDebugLog(@"Failed to recurse into directory '%@' - %@",
 			_currentFilePath, [NSError _last]);
 		    }
 		}
@@ -2944,7 +2944,7 @@ static inline void gsedRelease(GSEnumeratedDirectory X)
 	  s = [NSString stringWithFormat: @"cannot copy file type '%@'",
 	    fileType];
 	  ASSIGN(_lastError, s);
-	  NSLog(@"%@: %@", sourceFile, s);
+	  NSDebugLog(@"%@: %@", sourceFile, s);
 	  continue;
 	}
       [self changeFileAttributes: attributes atPath: destinationFile];
@@ -3639,7 +3639,7 @@ static NSSet	*fileKeys = nil;
     }
   if (count >= 2)
     {
-      NSLog(@"Warning ... key '%@' not handled", key);
+      NSDebugLog(@"Warning ... key '%@' not handled", key);
     }
   return nil;
 }
diff --git a/Source/NSPropertyList.m b/Source/NSPropertyList.m
index 3d1880347..27ef1dff3 100644
--- a/Source/NSPropertyList.m
+++ b/Source/NSPropertyList.m
@@ -2622,7 +2622,7 @@ GSPropertyListMake(id obj, NSDictionary *loc, BOOL xml,
             }
           
           if (length - index > 2
-              && bytes[index] == '<' && bytes[index+1] == '?')
+            && bytes[index] == '<' && bytes[index+1] == '?')
             {
               // It begins with '<?' so it is xml
               format = NSPropertyListXMLFormat_v1_0;
@@ -2630,7 +2630,7 @@ GSPropertyListMake(id obj, NSDictionary *loc, BOOL xml,
           else
             {
               // Assume openstep format unless we find otherwise.
-          format = NSPropertyListOpenStepFormat;
+              format = NSPropertyListOpenStepFormat;
             }
         }
     }
diff --git a/Source/NSUserDefaults.m b/Source/NSUserDefaults.m
index 5e53266a9..f13159a9b 100644
--- a/Source/NSUserDefaults.m
+++ b/Source/NSUserDefaults.m
@@ -88,6 +88,7 @@ static NSString		*GSPrimaryDomain = @"GSPrimaryDomain";
 static NSString		*defaultsFile = @".GNUstepDefaults";
 
 static NSUserDefaults	*sharedDefaults = nil;
+static NSDictionary     *argumentsDictionary = nil;
 static NSMutableString	*processName = nil;
 static NSRecursiveLock	*classLock = nil;
 
@@ -97,10 +98,10 @@ static NSRecursiveLock	*classLock = nil;
  * have no defaults available.
  */
 static BOOL		hasSharedDefaults = NO;
-/*
- * Caching some defaults.
+
+/* Caching some default flag values.  Until the standard defaults have
+ * been loaded, these values are taken from the process arguments.
  */
-static BOOL     parsingArguments = NO;
 static BOOL	flags[GSUserDefaultMaxFlag] = { 0 };
 
 /* An instance of the GSPersistentDomain class is used to encapsulate
@@ -244,6 +245,8 @@ updateCache(NSUserDefaults *self)
 	    }
 	}
 
+      /* NB the following flags are first set up, in the +initialize method.
+       */
       flags[GSMacOSXCompatible]
 	= [self boolForKey: @"GSMacOSXCompatible"];
       flags[GSOldStyleGeometry]
@@ -429,7 +432,7 @@ newLanguages(NSArray *oldNames)
  *** Private method definitions
  *************************************************************************/
 @interface NSUserDefaults (Private)
-- (NSDictionary*) _createArgumentDictionary;
++ (void) _createArgumentDictionary: (NSArray*)args;
 - (void) _changePersistentDomain: (NSString*)domainName;
 - (NSString*) _directory;
 - (BOOL) _lockDefaultsFile: (BOOL*)wasLocked;
@@ -547,6 +550,7 @@ newLanguages(NSArray *oldNames)
 {
   DESTROY(sharedDefaults);
   DESTROY(processName);
+  DESTROY(argumentsDictionary);
   DESTROY(classLock);
 }
 
@@ -554,6 +558,11 @@ newLanguages(NSArray *oldNames)
 {
   if (self == [NSUserDefaults class])
     {
+      CREATE_AUTORELEASE_POOL(pool);
+      NSEnumerator      *enumerator;
+      NSArray           *args;
+      NSString          *key;
+
       nextObjectSel = @selector(nextObject);
       objectForKeySel = @selector(objectForKey:);
       addSel = @selector(addEntriesFromDictionary:);
@@ -567,8 +576,67 @@ newLanguages(NSArray *oldNames)
       NSNumberClass = [NSNumber class];
       NSMutableDictionaryClass = [NSMutableDictionary class];
       NSStringClass = [NSString class];
-      classLock = [GSLazyRecursiveLock new];
       [self registerAtExit];
+
+      /* Initialise the defaults flags to take values from the
+       * process arguments.  These are otherwise set in updateCache()
+       * We do this early on so that the boolean argument settings can
+       * be used while parsing property list values of other args in
+       * the +_createArgumentDictionary: method.
+       */
+      args = [[NSProcessInfo processInfo] arguments];
+      enumerator = [[[NSProcessInfo processInfo] arguments] objectEnumerator];
+      [enumerator nextObject];	// Skip process name.
+      while (nil != (key = [enumerator nextObject]))
+        {
+          if ([key hasPrefix: @"-"] == YES && [key isEqual: @"-"] == NO)
+	    {
+              id        val;
+
+	      /* Anything beginning with a '-' is a defaults key and we
+               * must strip the '-' from it.
+               */
+	      key = [key substringFromIndex: 1];
+	      while (nil != (val = [enumerator nextObject]))
+                {
+                  if ([val hasPrefix: @"-"] == YES && [val isEqual: @"-"] == NO)
+                    {
+                      key = val;
+                    }
+                  else if ([key isEqualToString: @"GSMacOSXCompatible"])
+                    {
+                      flags[GSMacOSXCompatible] = [val boolValue];
+                    }
+                  else if ([key isEqualToString: @"GSOldStyleGeometry"])
+                    {
+                      flags[GSOldStyleGeometry] = [val boolValue];
+                    }
+                  else if ([key isEqualToString: @"GSLogSyslog"])
+                    {
+                      flags[GSLogSyslog] = [val boolValue];
+                    }
+                  else if ([key isEqualToString: @"GSLogThread"])
+                    {
+                      flags[GSLogThread] = [val boolValue];
+                    }
+                  else if ([key isEqualToString: @"GSLogOffset"])
+                    {
+                      flags[GSLogOffset] = [val boolValue];
+                    }
+                  else if ([key isEqual: @"NSWriteOldStylePropertyLists"])
+                    {
+                      flags[NSWriteOldStylePropertyLists] = [val boolValue];
+                    }
+	        }
+	    }
+        }
+      /* The classLock must be created after setting up the flags[] array,
+       * so once it exists we know we can used them safely.
+       */
+      classLock = [NSRecursiveLock new];
+
+      [self _createArgumentDictionary: args];
+      DESTROY(pool);
     }
 }
 
@@ -804,19 +872,20 @@ newLanguages(NSArray *oldNames)
 
       if (nil == defs)
 	{
-	  const unsigned retryCount = 100;
-	  const NSTimeInterval retryInterval = 0.1;
-	  unsigned i;
+	  const unsigned        retryCount = 100;
+	  const NSTimeInterval  retryInterval = 0.1;
+	  unsigned              i;
+
 	  for (i = 0; i < retryCount; i++)
 	    {
-	      [NSThread sleepForTimeInterval:retryInterval];
+	      [NSThread sleepForTimeInterval: retryInterval];
 	      [classLock lock];
-	      defs = [sharedDefaults retain];
+	      defs = RETAIN(sharedDefaults);
 	      setup = hasSharedDefaults;
 	      [classLock unlock];
 	      if (YES == setup)
 		{
-		  NS_VALRETURN([defs autorelease]);
+		  NS_VALRETURN(AUTORELEASE(defs));
 		}
               RELEASE(defs);
 	    }
@@ -1117,7 +1186,6 @@ newLanguages(NSArray *oldNames)
   NSFileManager	*mgr = [NSFileManager defaultManager];
   NSRange	r;
   BOOL		flag;
-  NSDictionary *argumentsDictionary = nil;
 
   self = [super init];
 
@@ -1175,12 +1243,7 @@ newLanguages(NSArray *oldNames)
 
   // Create volatile defaults and add the Argument and the Registration domains
   _tempDomains = [[NSMutableDictionaryClass alloc] initWithCapacity: 10];
-  argumentsDictionary = [self _createArgumentDictionary];
-  if (nil != argumentsDictionary)
-    {
-      [_tempDomains setObject: argumentsDictionary
-		   forKey: NSArgumentDomain];
-    }
+  [_tempDomains setObject: argumentsDictionary forKey: NSArgumentDomain];
   [_tempDomains
     setObject: [NSMutableDictionaryClass dictionaryWithCapacity: 10]
     forKey: NSRegistrationDomain];
@@ -2132,9 +2195,18 @@ static BOOL isPlistObject(id o)
 BOOL
 GSPrivateDefaultsFlag(GSUserDefaultFlagType type)
 {
-  if (nil == sharedDefaults && NO == parsingArguments)
+  if (nil == classLock)
     {
-      [NSUserDefaults standardUserDefaults];
+      /* The order of +initialise of NSUserDefaults is such that our
+       * flags[] array is set up directly from the process arguments
+       * before classLock is created, so once * that variable exists
+       * this function may be used safely.
+       */
+      [NSUserDefaults class];
+      if (NO == hasSharedDefaults)
+        {
+          [NSUserDefaults standardUserDefaults];
+        }
     }
   return flags[type];
 }
@@ -2150,7 +2222,7 @@ NSDictionary *GSPrivateDefaultLocale()
   NSDictionary	        *locale = nil;
   NSUserDefaults        *defs = nil;
 
-  if (classLock == nil)
+  if (nil == classLock)
     {
       [NSUserDefaults standardUserDefaults];
     }
@@ -2177,26 +2249,16 @@ NSDictionary *GSPrivateDefaultLocale()
 
 @implementation NSUserDefaults (Private)
 
-- (NSDictionary*) _createArgumentDictionary
++ (void) _createArgumentDictionary: (NSArray*)args
 {
-  NSArray	*args;
   NSEnumerator	*enumerator;
   NSMutableDictionary *argDict = nil;
   BOOL		done;
   id		key, val;
 
   [classLock lock];
-  if (YES == parsingArguments)
-    {
-      [classLock unlock];
-      return nil;       // Prevent recursion
-    }
-  parsingArguments = YES;
-  [classLock unlock];
-  [_lock lock];
   NS_DURING
     {
-      args = [[NSProcessInfo processInfo] arguments];
       enumerator = [args objectEnumerator];
       argDict = [NSMutableDictionaryClass dictionaryWithCapacity: 2];
       [enumerator nextObject];	// Skip process name.
@@ -2204,40 +2266,35 @@ NSDictionary *GSPrivateDefaultLocale()
 
       while (done == NO)
         {
-          if ([key hasPrefix: @"-"] == YES && [key isEqual: @"-"] == NO)
+          /* Any value with a leading '-' may be the name of a default
+           * in the argument domain.
+           * NB. Testing on OSX shows that this includes a single '-'
+           * (where the key is an empty string), but GNUstep disallows
+           * en empty string as a key (so it can be a value).
+           */
+          if ([key hasPrefix: @"-"] == YES
+            && [key isEqual: @"-"] == NO)
 	    {
-	      NSString	*old = nil;
-
-	      /* anything beginning with a '-' is a defaults key and we
-               * must strip the '-' from it.
-               * As a special case, we leave the '- in place for '-GS...'
-               * and '--GS...' for backward compatibility.
+	      /* Strip the '-' before the defaults key, and get the
+               * corresponding value (the next argument).
                */
-	      if ([key hasPrefix: @"-GS"] == YES
-                || [key hasPrefix: @"--GS"] == YES)
-	        {
-	          old = key;
-	        }
 	      key = [key substringFromIndex: 1];
 	      val = [enumerator nextObject];
-	      if (val == nil)
-	        {            // No more args
-	          [argDict setObject: @"" forKey: key];		// arg is empty.
-	          if (old != nil)
-		    {
-		      [argDict setObject: @"" forKey: old];
-		    }
+	      if (nil == val)
+	        {
+                  /* No more arguments and no value ... arg is not set.
+                   */
 	          done = YES;
 	          continue;
 	        }
 	      else if ([val hasPrefix: @"-"] == YES
                 && [val isEqual: @"-"] == NO)
-	        {  // Yet another argument
-	          [argDict setObject: @"" forKey: key];		// arg is empty.
-	          if (old != nil)
-		    {
-		      [argDict setObject: @"" forKey: old];
-		    }
+	        {
+                  /* Value is actually an argument key ...
+                   * current key is not used (behavior matches OSX).
+                   * NB. GNUstep allows a '-' as the value for a default,
+                   * but OSX does not.
+                   */
 	          key = val;
 	          continue;
 	        }
@@ -2250,15 +2307,26 @@ NSDictionary *GSPrivateDefaultLocale()
 		     foreign environment. */
 	          NSObject *plist_val;
 
-	          NS_DURING
-		    {
-		      plist_val = [val propertyList];
-		    }
-	          NS_HANDLER
-		    {
-		      plist_val = val;
-		    }
-	          NS_ENDHANDLER
+                  NS_DURING
+                    {
+                      NSData		        *data;
+
+                      data = [val dataUsingEncoding: NSUTF8StringEncoding];
+                      plist_val = [NSPropertyListSerialization
+                        propertyListFromData: data
+                        mutabilityOption: NSPropertyListMutableContainers
+                        format: 0
+                        errorDescription: 0];
+                      if (nil == plist_val)
+                        {
+                          plist_val = val;
+                        }
+                    }
+                  NS_HANDLER
+                    {
+                      plist_val = val;
+                    }
+                  NS_ENDHANDLER
 
 	          /* Make sure we don't crash being caught adding nil to
                      a dictionary. */
@@ -2268,29 +2336,20 @@ NSDictionary *GSPrivateDefaultLocale()
 		    }
 
 	          [argDict setObject: plist_val  forKey: key];
-	          if (old != nil)
-		    {
-		      [argDict setObject: plist_val  forKey: old];
-		    }
 	        }
 	    }
           done = ((key = [enumerator nextObject]) == nil);
         }
-      [classLock lock];
-      parsingArguments = NO;
+      argumentsDictionary = [argDict copy];
       [classLock unlock];
-      [_lock unlock];
     }
   NS_HANDLER
     {
-      [classLock lock];
-      parsingArguments = NO;
+      argumentsDictionary = [NSDictionary new];
       [classLock unlock];
-      [_lock unlock];
       [localException raise];
     }
   NS_ENDHANDLER
-  return argDict;
 }
 
 - (void) _changePersistentDomain: (NSString*)domainName