NSString memory problems

Discussion in 'Mac Programming' started by ranguvar, Jan 3, 2010.

  1. ranguvar macrumors 6502

    Joined:
    Sep 18, 2009
    #1
    Hi,

    I wrote a small app in Obj-C that uses garbage collection. I combined to strings like this:

    Code:
    // Initialize two strings.
    NSString *s1 = [[NSString alloc] initWithString:@"apple"];
    NSString *s2 = [[NSString alloc] initWithString:@"juice"];
    
    // Combine them into a third string.
    NSString *s3 = [s1 stringByAppendingString:s2];
    
    // Make the first two strings ready to be recycled by GC.
    s1 = nil;
    s2 = nil;
    But somehow, s3 ends up being (null). Why? Doesn't the stringByAppendingString: method copy the values of s1 and s2 into s3?

    If I'm wrong, how else could I free the memory of s1 and s2 without losing s3?
     
  2. gnasher729 macrumors P6

    gnasher729

    Joined:
    Nov 25, 2005
    #2
    How do you know that s3 is nil? I mean: Tell is _exactly_ how you know this.
     
  3. mdeh macrumors 6502

    Joined:
    Jan 3, 2009
    #3

    I suspect that your NSLog statement is incorrect, not your assumption that s3 is "NULL".

    So, the key **might** be this.

    Code:
    NSLog(@"%@", s3);
    
     
  4. MrFusion macrumors 6502a

    Joined:
    Jun 8, 2005
    Location:
    West-Europe
    #4
    The @"%@" is redundant. NSLog takes NSString as argument.
    For all we know, he might have use the debugger to conclude that s3 is null.
     
  5. kpua macrumors 6502

    Joined:
    Jul 25, 2006
    #5
    The @"%@" is NOT redundant. It's necessary to avoid the format string security vulnerability. There's no chance of it here, but you should always be in the habit of doing this to avoid the problem in situations where it could.

    As to the original question. The only way s3 could be nil is if s1 is nil. The only way I can see that happening is if the +alloc failed, which would only happen if the heap's address space is completely exhausted. So, I also expect there's something fishy here.
     
  6. MrFusion macrumors 6502a

    Joined:
    Jun 8, 2005
    Location:
    West-Europe
    #6
    Could you elaborate, for educational purposes?
     
  7. Guiyon macrumors 6502a

    Joined:
    Mar 19, 2008
    Location:
    North Shore, MA
    #7
    Say for example you have some string 'foo'. Assume you don't know the contents of the string (it is defined below simply as a vulnerability example).
    Code:
    NSString *foo = @"This is %d some %@ format %s string";
    Code:
    NSLog( foo );
    When you call NSLog using that string you have a problem (see above). There are 3 format placeholders but no arguments so you end up with undefined behavior. The best case scenario you end up with is printing garbage or a crash. The worst case scenario you end up giving someone a free pass to inject malicious code.

    If you call
    Code:
    NSLog( @"%@", foo );
    only the first string is parsed for any format specifiers, in which case it finds a single '%@', which is matched with the 'foo'. 'foo' does not undergo further processing so you just end up printing the contents of 'foo' as you initially desired.
     
  8. chown33 macrumors 604

    Joined:
    Aug 9, 2009
    #8
  9. ranguvar thread starter macrumors 6502

    Joined:
    Sep 18, 2009
    #9
    I tried
    Code:
    NSLog(@"string: %@", s3);
    which gave the output
    Code:
    string: (null)
     
  10. robbieduncan Moderator emeritus

    robbieduncan

    Joined:
    Jul 24, 2002
    Location:
    London
    #10
    Where do you have this statement? In the same method? A different method? When is it called? Within the same runloop invocation? Later? s3 is autoreleased so available for GC...
     
  11. chown33 macrumors 604

    Joined:
    Aug 9, 2009
    #11
    Please post a complete compilable example, not just isolated code fragments.

    I put the following into -init of MyDocument.m in a document-based Cocoa app compiled with GC Required. It works fine.

    Code:
    {
      NSString *s1 = [[NSString alloc] initWithString:@"apple"];
      NSString *s2 = [[NSString alloc] initWithString:@"juice"];
    
      NSString *s3 = [s1 stringByAppendingString:s2];
    
      s1 = nil;
      s2 = nil;
    
      NSLog( @"%@", s3 );
    }
    
     
  12. GorillaPaws macrumors 6502a

    GorillaPaws

    Joined:
    Oct 26, 2003
    Location:
    Richmond, VA
    #12
    HA! That made my Sunday.
     
  13. jared_kipe macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #13
    Odd. Does it still fail if you remove the two s1=nil; statements? Did you copy and paste that code? No way there is a typo like say s3 isn't really s3 at some place?

    How about..
    Code:
    {
      NSString *s1 = @"apple";
      NSString *s2 = @"juice";
    
      NSString *s3 = [s1 stringByAppendingString:s2];
    //...
      NSLog( @"%@", s3 );
    }
    
    Or even..
    Code:
    {
      NSString *s3 = [@"apple" stringByAppendingString:@"juice"];
      NSLog( @"%@", s3 );
    }
    
    or
    Code:
    {
      NSString *s1 = @"apple";
      NSString *s2 = @"juice";
    
      NSLog( @"%@", [s1 stringByAppendingString:s2] );
    }
    


    What headers are you including?
     
  14. rossipoo macrumors regular

    Joined:
    Jun 7, 2009
    #14
    NSLog is not intended for normal output anyway; it should only be used for debugging purposes, and should not be included release builds.
     
  15. MrFusion macrumors 6502a

    Joined:
    Jun 8, 2005
    Location:
    West-Europe
    #15
    Indeed. Great site!
     
  16. jared_kipe macrumors 68030

    jared_kipe

    Joined:
    Dec 8, 2003
    Location:
    Seattle
    #16
    Why not? You could use it to display an error semi gracefully, meaning the end user doesn't have to be aware of it. But when they start calling tech support you can point them to the console and see if they are getting any of your errors.
     
  17. rossipoo macrumors regular

    Joined:
    Jun 7, 2009
    #17
    A fair question; software with known problems is shipped all the time.
     

Share This Page