Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Fix miscellaneous static analyzer warnings #7670

Merged
merged 4 commits into from
Jan 11, 2017
Merged

Fix miscellaneous static analyzer warnings #7670

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4adb075
[ios, macos] Fixed static analyzer warnings
1ec5 Jan 11, 2017
37ef97a
[ios] Fixed static analyzer warnings
1ec5 Jan 11, 2017
cce1231
[macos] Fixed static analyzer warnings
1ec5 Jan 11, 2017
d7e63d0
[ios, macos] Two pointer-equal images are object-equal by definition
1ec5 Jan 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions platform/darwin/src/MGLNetworkConfiguration.m
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#import "MGLNetworkConfiguration.h"
#import "NSProcessInfo+MGLAdditions.h"

@implementation MGLNetworkConfiguration

Expand All @@ -12,9 +11,6 @@ + (void)load {
}

+ (instancetype)sharedManager {
if (NSProcessInfo.processInfo.mgl_isInterfaceBuilderDesignablesAgent) {
return nil;
}
static dispatch_once_t onceToken;
static MGLNetworkConfiguration *_sharedManager;
Copy link
Contributor Author

@1ec5 1ec5 Jan 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

+[MGLAccountManager sharedManager] bails in IB because MGLAccountManager spins up lots of other machinery, including MGLMapboxEvents. MGLNetworkConfiguration is intentionally isolated from anything else, so initializing the singleton within IB is no big deal.

void (^setupBlock)() = ^{
Expand Down
2 changes: 1 addition & 1 deletion platform/darwin/src/MGLRasterSource.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ - (instancetype)initWithIdentifier:(NSString *)identifier tileURLTemplates:(NS_A
if (self = [super initWithIdentifier:identifier tileURLTemplates:tileURLTemplates options:options]) {
mbgl::Tileset tileSet = MGLTileSetFromTileURLTemplates(tileURLTemplates, options);

uint16_t tileSize;
uint16_t tileSize = MGLRasterSourceRetinaTileSize;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had somehow forgotten that primitive-typed stack variables are uninitialized in Objective-C just as they are in C. ARC nils out only object pointers on the stack.

if (NSNumber *tileSizeNumber = options[MGLTileSourceOptionTileSize]) {
if (![tileSizeNumber isKindOfClass:[NSNumber class]]) {
[NSException raise:NSInvalidArgumentException
Expand Down
4 changes: 1 addition & 3 deletions platform/darwin/src/MGLShape.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ - (BOOL)isEqual:(id)other

- (NSUInteger)hash
{
NSUInteger hash;
hash += _title.hash;
hash += _subtitle.hash;
NSUInteger hash = _title.hash + _subtitle.hash;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a safe way to compute the hash?

Copy link
Contributor

Choose a reason for hiding this comment

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

A XOR over the values could improve it but I'm ok with it because this is an abstract class and can't be used directly. All concrete subclasses include the hash of the GeoJSON dictionary.

Copy link
Contributor Author

@1ec5 1ec5 Jan 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This is a fairly conventional implementation, as far as I know. The only hard requirement for a -hash implementation is that two equal objects (as in -isEqual:) must have the same hash. But feel free to improve on the various -hash implementations in this SDK in a separate PR.

#if !TARGET_OS_IPHONE
hash += _toolTip.hash;
#endif
Expand Down
8 changes: 2 additions & 6 deletions platform/ios/src/MGLAnnotationImage.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,11 @@ - (BOOL)isEqual:(id)other {

return ((!_reuseIdentifier && !otherAnnotationImage.reuseIdentifier) || [_reuseIdentifier isEqualToString:otherAnnotationImage.reuseIdentifier])
&& _enabled == otherAnnotationImage.enabled
&& ((!_image && !otherAnnotationImage.image) || [UIImagePNGRepresentation(_image) isEqualToData:UIImagePNGRepresentation(otherAnnotationImage.image)]);
&& ((!_image && !otherAnnotationImage.image) || [_image isEqual:otherAnnotationImage.image]);
Copy link
Contributor Author

@1ec5 1ec5 Jan 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@frederoni, any reason why comparing two UIImages for object equality isn’t good enough? (There’s a similar comparison of NSImages’ TIFF representations on the macOS side.)

Copy link
Contributor Author

@1ec5 1ec5 Jan 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I see that testAnnotationImage() is failing as a result of my change. It’s true that drawing the image to a new context could make the two images unequal, but there are a number of reasons that would be the case anyways (such as different color profiles). In general, I don’t think a class’s -isEqual: implementation should be more thorough than the -isEqual: implementations of its ivars.

Edit: I misread the test. Looks like it’s doing something pretty ordinary.

}

- (NSUInteger)hash {
NSUInteger hash;
hash += [_reuseIdentifier hash];
hash += _enabled;
hash += [_image hash];
return hash;
return _reuseIdentifier.hash + _enabled + _image.hash;
}

- (void)setImage:(UIImage *)image {
Expand Down
6 changes: 1 addition & 5 deletions platform/macos/src/MGLAnnotationImage.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,7 @@ - (BOOL)isEqual:(id)other {
}

- (NSUInteger)hash {
NSUInteger hash;
hash += [_reuseIdentifier hash];
hash += _selectable;
hash += [_image hash];
return hash;
return _reuseIdentifier.hash + @(_selectable).hash + _image.hash;
}

@end
2 changes: 1 addition & 1 deletion platform/macos/src/NSImage+MGLAdditions.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ - (nullable instancetype)initWithMGLSpriteImage:(const mbgl::SpriteImage *)sprit
std::string png = encodePNG(spriteImage->image);
NSData *data = [[NSData alloc] initWithBytes:png.data() length:png.size()];
NSBitmapImageRep *rep = [NSBitmapImageRep imageRepWithData:data];
if ([self initWithSize:NSMakeSize(spriteImage->getWidth(), spriteImage->getHeight())]) {
if (self = [self initWithSize:NSMakeSize(spriteImage->getWidth(), spriteImage->getHeight())]) {
[self addRepresentation:rep];
[self setTemplate:spriteImage->sdf];
}
Expand Down