google / s2geometry

Computational geometry and spatial indexing on the sphere
http://s2geometry.io/
Apache License 2.0
2.29k stars 302 forks source link

Fix undefined sanitizer issue in compact_array #284

Closed MBkkt closed 1 year ago

MBkkt commented 1 year ago

Also I think really correct code, should start with non zero capacity (so correct capacity can be seted in ctor)

jmr commented 1 year ago

Also I think really correct code, should start with non zero capacity (so correct capacity can be seted in ctor)

Why? Feel free to send another PR if you think there are more improvements.

MBkkt commented 1 year ago

Why? Feel free to send another PR if you think there are more improvements

Because we always have some capacity on "stack". Same as for sso string.

jmr commented 1 year ago

Because we always have some capacity on "stack". Same as for sso string.

I'm not sure what you mean. There's already inline SSO-like space allocated: https://github.com/google/s2geometry/blob/e15d00b21e9bbc252e436c243710cb9712465814/src/s2/util/gtl/compact_array.h#L127

Send a PR to make it clear.

MBkkt commented 1 year ago

I mean sso string after default ctor have non zero capacity. Why same not possible for this array?

jmr commented 1 year ago

Anything is possible. Send a PR if you think it can be improved.