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

OCPartialMockObject static analyzer warning #543

Open
LowAmmo opened this issue Nov 8, 2024 · 2 comments
Open

OCPartialMockObject static analyzer warning #543

LowAmmo opened this issue Nov 8, 2024 · 2 comments

Comments

@LowAmmo
Copy link

LowAmmo commented Nov 8, 2024

In Xcode if running "Analyze" on a target that includes OCMock, getting a warning about OCPartialMockObject#setupForwarderForSelector. On the last line, the call to class_addMethod about the possibility of passing a null parameter as the 3rd parameter.

class_addMethod(subclass, aliasSelector, originalIMP, types);

 - (void)setupForwarderForSelector:(SEL)sel
 {
     SEL aliasSelector = OCMAliasForOriginalSelector(sel);
     if(class_getInstanceMethod(object_getClass(realObject), aliasSelector) != NULL)
         return;
 
     Method originalMethod = class_getInstanceMethod(mockedClass, sel);
     /* Might be NULL if the selector is forwarded to another class */
     IMP originalIMP = (originalMethod != NULL) ? method_getImplementation(originalMethod) : NULL;
     const char *types = (originalMethod != NULL) ? method_getTypeEncoding(originalMethod) : NULL;
     // TODO: check the fallback implementation is actually sufficient
     if(types == NULL)
         types = ([[mockedClass instanceMethodSignatureForSelector:sel] fullObjCTypes]);
 
     Class subclass = object_getClass([self realObject]);
     IMP forwarderIMP = [mockedClass instanceMethodForwarderForSelector:sel];
     class_replaceMethod(subclass, sel, forwarderIMP, types);
     class_addMethod(subclass, aliasSelector, originalIMP, types);  // <-- Possible for originalIMP to be nil
 }

I'm not a pro at everything OCMock does, but I'm guessing we probably at least don't want to call class_addMethod if originalIMP is NULL.

Unsure if it would make sense to not bother calling class_replaceMethod also...?

This isn't currently causing us any runtime issues - I just like to have 0 warnings in our code if we can.

-Thanks!

LowAmmo added a commit to LowAmmo/ocmock that referenced this issue Nov 12, 2024
* Issue erikdoe#543 about potential nil passed to class_addMethod()

OCPartialMockObject#setupForwarderForSelector
* Modified to just return if there is no existing implementation for the method
@LowAmmo
Copy link
Author

LowAmmo commented Nov 20, 2024

Created PR #544 to address this

@erikdoe
Copy link
Owner

erikdoe commented Nov 23, 2024

I've commented on the PR now.

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

No branches or pull requests

2 participants