Skip to content
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.

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

[TIMOB-18028] iOS: Fix all memory leaks detected in static analyzer #6350

Closed
wants to merge 2 commits into from

Conversation

cheekiatng
Copy link
Contributor

Static analyzer detects potential memory leak of objects. These should be checked and fixed when necessary.
JIRA: https://jira.appcelerator.org/browse/TIMOB-18028

2 errors remain
@ingo
Copy link
Contributor

ingo commented Nov 14, 2014

Thanks! How should this be tested?

@@ -29,7 +29,7 @@ @implementation GeolocationCallback

-(id)initWithCallback:(KrollCallback*)callback_ context:(id<TiEvaluator>)context_
{
if (self = [super init])
if (self = [[super init] autorelease])
Copy link
Contributor

Choose a reason for hiding this comment

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

Do NOT autorelease in an init function. ALWAYS balance releases with allocations.
You should ignore this analyzer warning. The GeolocationCallback objects implement APSHTTPRequestDelegate. Since the HTTPClient holds a weak reference to the delegate the delegate has to stay around till the request completes. The delegate will call autorelease on itself in either onLoad or onError (since one of them is guaranteed to be called). You can just put a comment in code that this analyzer warning can be ignored

@vishalduggal
Copy link
Contributor

Code reviewed. Please address comments.

@@ -192,7 +192,7 @@ -(NSNumber*)copy:(id)args
NSRange replacement = NSMakeRange(offset, MIN(MIN(sourceLength, [data length]-offset), [[sourceBuffer data] length]-sourceOffset));
[data replaceBytesInRange:replacement withBytes:(source+sourceOffset)];

return NUMINT(replacement.length);
return [NUMINT(replacement.length) retain];
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not retain. You can ignore this analyzer warning

@vishalduggal
Copy link
Contributor

PR has been cherry-picked to the 3_5_0_64-bit branch which will be merged back to master soon.
PR #6363
Closing this PR

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

Successfully merging this pull request may close these issues.

None yet

3 participants