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

Commit

Permalink
[ios, macos] Use std::vector for MGLMultiPoint coordinate storage (#6890
Browse files Browse the repository at this point in the history 
)

Refactor the previous C array based implementation to use C++ std::vector. This is done to avoid potential safety and security issues with the previous approach.
  • Loading branch information
@boundsj
boundsj authored Nov 14, 2016
1 parent 96a7a44 commit 44e3988
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 92 deletions.
23 changes: 16 additions & 7 deletions platform/darwin/src/MGLMultiPoint.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,26 @@ NS_ASSUME_NONNULL_BEGIN
cause the shape to be redrawn if it is currently visible on the map.
@param range The range of points to update. The `location` field indicates the
first point you are replacing, with `0` being the first point, `1` being
the second point, and so on. The `length` field indicates the number of
points to update. You can append to an existing array of coordinates
by specifying a range with a `location` at the end of the existing array
and/or a `length` which extends past the end of the existing array. The array
in _`coords`_ must be equal in number to the length of the range.
first point you are replacing, with `0` being the first point, `1` being
the second point, and so on. The `length` field indicates the number of
points to update. The array in _`coords`_ must be equal in number to the
length of the range. If you want to append to the existing coordinates
array use `-[MGLMultiPoint appendCoordinates:count:]`.
@param coords The array of coordinates defining the shape. The data in this
array is copied to the object.
array is copied to the object.
*/
- (void)replaceCoordinatesInRange:(NSRange)range withCoordinates:(CLLocationCoordinate2D *)coords;

/**
Appends one or more coordinates for the shape, which will instantaneously
cause the shape to be redrawn if it is currently visible on the map.
@param coords The array of coordinates to add to the shape. The data in this
array is copied to the new object.
@param count The number of items in the `coords` array.
*/
- (void)appendCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count;

@end

NS_ASSUME_NONNULL_END
100 changes: 39 additions & 61 deletions platform/darwin/src/MGLMultiPoint.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@

@implementation MGLMultiPoint
{
size_t _count;
MGLCoordinateBounds _bounds;
std::vector<CLLocationCoordinate2D> _coordinates;
}

- (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
Expand All @@ -24,105 +24,83 @@ - (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUI

if (self)
{
[self setupWithCoordinates:coords count:count];
NSAssert(count > 0, @"A multipoint must have coordinates");
_coordinates = { coords, coords + count };
[self computeBounds];
}

return self;
}

- (void)setupWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
{
if (_coordinates) free(_coordinates);

_count = count;
_coordinates = (CLLocationCoordinate2D *)malloc(_count * sizeof(CLLocationCoordinate2D));

mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();

for (NSUInteger i = 0; i < _count; i++)
{
_coordinates[i] = coords[i];
bounds.extend(mbgl::LatLng(coords[i].latitude, coords[i].longitude));
}

_bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
}

- (void)dealloc
{
free(_coordinates);
}

- (CLLocationCoordinate2D)coordinate
{
assert(_count > 0);

return CLLocationCoordinate2DMake(_coordinates[0].latitude, _coordinates[0].longitude);
NSAssert([self pointCount] > 0, @"A multipoint must have coordinates");
return _coordinates.at(0);
}

- (NSUInteger)pointCount
{
return _count;
return _coordinates.size();
}

+ (NS_SET_OF(NSString *) *)keyPathsForValuesAffectingPointCount
{
return [NSSet setWithObjects:@"coordinates", nil];
}

- (CLLocationCoordinate2D *)coordinates
{
return _coordinates.data();
}

- (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range
{
if (range.location + range.length > _count)
if (range.location + range.length > [self pointCount])
{
[NSException raise:NSRangeException
format:@"Invalid coordinate range %@ extends beyond current coordinate count of %zu",
NSStringFromRange(range), _count];
NSStringFromRange(range), [self pointCount]];
}

NSUInteger index = 0;
std::copy(_coordinates.begin() + range.location, _coordinates.begin() + NSMaxRange(range), coords);
}

for (NSUInteger i = range.location; i < range.location + range.length; i++)
{
coords[index] = _coordinates[i];
index++;
}
- (void)appendCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
{
[self willChangeValueForKey:@"coordinates"];
_coordinates.insert(_coordinates.end(), count, *coords);
[self computeBounds];
[self didChangeValueForKey:@"coordinates"];
}

- (void)replaceCoordinatesInRange:(NSRange)range withCoordinates:(CLLocationCoordinate2D *)coords
{
if ((coords >= _coordinates && coords < _coordinates + _count) ||
(coords + range.length >= _coordinates && coords + range.length < _coordinates + _count))
{
[NSException raise:NSRangeException format:@"Reuse of existing coordinates array %p not supported", coords];
}
else if (range.length == 0)
if (range.length == 0)
{
[NSException raise:NSRangeException format:@"Empty coordinate range %@", NSStringFromRange(range)];
return;
}
else if (range.location > _count)

if (NSMaxRange(range) > _coordinates.size())
{
[NSException raise:NSRangeException
format:@"Invalid range %@ for existing coordinate count %zu",
NSStringFromRange(range), _count];
NSStringFromRange(range), [self pointCount]];
}

[self willChangeValueForKey:@"coordinates"];
if (NSMaxRange(range) <= _count)
{
// replacing existing coordinate(s)
memcpy(_coordinates + range.location, coords, range.length * sizeof(CLLocationCoordinate2D));
}
else
std::copy(coords, coords + range.length, _coordinates.begin() + range.location);
[self computeBounds];
[self didChangeValueForKey:@"coordinates"];
}

- (void)computeBounds
{
mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();
for (auto coordinate : _coordinates)
{
// appending new coordinate(s)
NSUInteger newCount = NSMaxRange(range);
CLLocationCoordinate2D *newCoordinates = (CLLocationCoordinate2D *)malloc(newCount * sizeof(CLLocationCoordinate2D));
memcpy(newCoordinates, _coordinates, fmin(_count, range.location) * sizeof(CLLocationCoordinate2D));
memcpy(newCoordinates + range.location, coords, range.length * sizeof(CLLocationCoordinate2D));
[self setupWithCoordinates:newCoordinates count:newCount];
free(newCoordinates);
bounds.extend(mbgl::LatLng(coordinate.latitude, coordinate.longitude));
}
[self didChangeValueForKey:@"coordinates"];
_bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
}

- (MGLCoordinateBounds)overlayBounds
Expand All @@ -144,7 +122,7 @@ - (BOOL)intersectsOverlayBounds:(MGLCoordinateBounds)overlayBounds
- (NSString *)description
{
return [NSString stringWithFormat:@"<%@: %p; count = %lu; bounds = %@>",
NSStringFromClass([self class]), (void *)self, (unsigned long)_count, MGLStringFromCoordinateBounds(_bounds)];
NSStringFromClass([self class]), (void *)self, (unsigned long)[self pointCount], MGLStringFromCoordinateBounds(_bounds)];
}

@end
41 changes: 17 additions & 24 deletions platform/darwin/src/MGLPointCollection.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@

@implementation MGLPointCollection
{
size_t _count;
MGLCoordinateBounds _bounds;
std::vector<CLLocationCoordinate2D> _coordinates;
}

+ (instancetype)pointCollectionWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUInteger)count
Expand All @@ -22,50 +22,43 @@ - (instancetype)initWithCoordinates:(CLLocationCoordinate2D *)coords count:(NSUI
self = [super init];
if (self)
{
_count = count;
_coordinates = (CLLocationCoordinate2D *)malloc(_count * sizeof(CLLocationCoordinate2D));

_coordinates = { coords, coords + count };
mbgl::LatLngBounds bounds = mbgl::LatLngBounds::empty();

for (NSUInteger i = 0; i < count; i++)
for (auto coordinate : _coordinates)
{
_coordinates[i] = coords[i];
bounds.extend(mbgl::LatLng(coords[i].latitude, coords[i].longitude));
bounds.extend(mbgl::LatLng(coordinate.latitude, coordinate.longitude));
}

_bounds = MGLCoordinateBoundsFromLatLngBounds(bounds);
}
return self;
}

- (NSUInteger)pointCount
{
return _count;
return _coordinates.size();
}

- (CLLocationCoordinate2D *)coordinates
{
return _coordinates.data();
}

- (CLLocationCoordinate2D)coordinate
{
assert(_count > 0);

return CLLocationCoordinate2DMake(_coordinates[0].latitude, _coordinates[0].longitude);
NSAssert([self pointCount] > 0, @"A multipoint must have coordinates");
return _coordinates.at(0);
}

- (void)getCoordinates:(CLLocationCoordinate2D *)coords range:(NSRange)range
{
if (range.location + range.length > _count)
if (range.location + range.length > [self pointCount])
{
[NSException raise:NSRangeException
format:@"Invalid coordinate range %@ extends beyond current coordinate count of %zu",
NSStringFromRange(range), _count];
}

NSUInteger index = 0;

for (NSUInteger i = range.location; i < range.location + range.length; i++)
{
coords[index] = _coordinates[i];
index++;
NSStringFromRange(range), [self pointCount]];
}

std::copy(_coordinates.begin() + range.location, _coordinates.begin() + NSMaxRange(range), coords);
}

- (MGLCoordinateBounds)overlayBounds
Expand Down Expand Up @@ -104,7 +97,7 @@ - (NSDictionary *)geoJSONDictionary
- (NSString *)description
{
return [NSString stringWithFormat:@"<%@: %p; count = %lu>",
NSStringFromClass([self class]), (void *)self, (unsigned long)_count];
NSStringFromClass([self class]), (void *)self, (unsigned long)[self pointCount]];
}

@end
Expand Down

0 comments on commit 44e3988

Please sign in to comment.