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

WIP: Clean memory sanitizer warnings around GEOS code #265

Closed
wants to merge 3 commits into
base: svn-trunk
from

Conversation

Projects
None yet
2 participants
@Algunenano
Copy link
Member

Algunenano commented Jul 5, 2018

Shut up some of the warnings I get when running unit tests with -fsanitizer=memory

@@ -378,6 +378,8 @@ static char * lwdouble_to_dms(double val, const char *pos_dir_symbol, const char

/* Allocate space for the result. Leave plenty of room for excess digits, negative sign, etc.*/
result = (char*)lwalloc(format_length + WORK_SIZE);
memset(result, 0, format_length + WORK_SIZE);

This comment has been minimized.

@dbaston

dbaston Jul 5, 2018

Member

Do you think we should add a lwcalloc function that delegates to calloc on the lwgeom side and palloc0 on the postgis side?

This comment has been minimized.

@Algunenano

Algunenano Jul 5, 2018

Author Member

I've thought about that and it might help, but at the end of of the day if you don't use memset you probably won't use lwcalloc either.

Also, this case looks like a false positive as I don't see any way for the memory outside of the set string to be read, but since it's an output function and memset shouldn't any noticeable impact in performance I've decided to set it and make the sanitizer output clearer.

This comment has been minimized.

@dbaston

dbaston Jul 5, 2018

Member

grep -R -B1 "memset(" --include "*.c" | grep alloc | wc shows 20 or so usages. Seems worth it to me.

This comment has been minimized.

@Algunenano

Algunenano Jul 5, 2018

Author Member

Most of those seem uses to set simple structs, but I wouldn't oppose for it to be added in a followup issue.

@Algunenano

This comment has been minimized.

Copy link
Member Author

Algunenano commented Jul 6, 2018

@Algunenano Algunenano force-pushed the Algunenano:sanitizer_memory_geos branch from e2aa181 to 8fa848f Jul 6, 2018

@strk strk closed this in ec51dc1 Jul 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.