1

I am almost certain that I don't have a leak in this code, yet the Xcode analyzer reports that there is a "potential" leak (Xcode 4.6.1).

+ (MySHA1hash *)sha1HashWithHashBytes:(unsigned char *)hash length:(unsigned int)length;
{
   return [[[MySHA1hash alloc] initWithHashBytes:hash length:length] autorelease];
}

XCode analyzer error screenshot

If the problem is that Xcode is reporting a false positive, I would like to figure out how to structure the code in a way to silence the warning.

It is also the possible that I am leaking in a way I don't understand, but If someone can see how I am actually leaking I would be glad to get that feedback as well.

This must have something to do with the init functions I call, because if I simply replace initWithHashBytes with init, then the leak is no longer reported. To that end I also include the body of initWithHashBytes.

- (id)initWithHashBytes:(unsigned char *)hash length:(unsigned int)length
{
    if (hash != nil && length <= SHA_DIGEST_LENGTH) {
        NSData *data = [NSData dataWithBytes:hash length:length];
        self = [self initWithHash:data];
    }
    else {
        self = nil;
    }
    return self;
}

- (id)initWithHash:(NSData *)hash
{
    if ([hash length] <= SHA_DIGEST_LENGTH && (self = [super init]) != nil) {
        finished = YES;
        [hash getBytes:sha_Result];
        hashValue = [NSNumber numberWithInt:[hash hash]];
    }
    else {
        self = nil;
    }
    return self;
}
bneely
  • 9,083
  • 4
  • 38
  • 46
John Bowers
  • 1,695
  • 1
  • 16
  • 26
  • 1
    This is not a false positive. Hint: the word "potential" is included in the report. –  Mar 27 '13 at 19:31
  • 1
    Ok, maybe I was getting down votes because of the previous title of the question. I have changed that. I still think the question is very reasonable. How do you silence an error / warning that you don't believe to be an correct. – John Bowers Mar 27 '13 at 19:43
  • I've generally found that the best way to "silence" analyzer complaints is to fix the problems. – Hot Licks Mar 27 '13 at 19:53
  • Well, That is probably true. I guess I am under the assumption that no analyzer is perfect. There is probably a reason that it is complaining, and there is a good chance it is right (as in this case it was), but I also assume it is possible that a static analyzer can be incorrect. – John Bowers Mar 27 '13 at 20:05
  • There are cases (such a misnaming a method as "new...") that can confuse the analyzer and cause it to report a leak that isn't present, but they've always been "real" errors, in my experience, just not always "real" leaks. – Hot Licks Mar 27 '13 at 21:44

3 Answers3

6

The line

self = nil;

in initWithHashBytes: (and initWithHash:) is the issue. You are allocating an object, but if you return nil from from initWithHashBytes:, that object will be leaked, because you'll call autorelease on nil rather than on the object you allocated.

Release self before you return nil and all should be good.

Seamus Campbell
  • 17,816
  • 3
  • 52
  • 60
4

In this particular case there was obviously an error that needed to be fixed, but I have seen at times a need to suppress warnings that are completely understood to be non problems (i.e. a leak reported that is not actually a leak).

This is what I expected to need to do here, but it turned out that there was an actual leak. So I am glad it got fixed. I immediately found another problem that was a clear an unmistakable "false positive" (I know that the error is reported as a "potential leak" so in reality it isn't a false positive, but it doesn't mean I want to see it in the report every time I run the analyzer).

Because of this I still had the question of how to suppress these warnings. It turns out you can easily wrap code that you want the analyzer to bypass in a ifdef check for __clang_analyzer.

#ifndef __clang_analyzer__
  ... code you want to ignore ...
#endif

There was a very good write up on this here.

John Bowers
  • 1,695
  • 1
  • 16
  • 26
1

You are missing a [self release] before self = nil.

The object you get from alloc has a reference count of +1 which needs to be balanced by a call to release or autorelease. For the case where you return your actual object from sha1HashWithHashBytes:length: the autorelease in that class method takes care of everything.

For the case you return nil your init method is the last one that has a reference to that allocated object so it has to release it.

Sven
  • 22,475
  • 4
  • 52
  • 71