This is an archive of the discontinued LLVM Phabricator instance.

Define OFF_T as 64-bit integer on NetBSD
ClosedPublic

Authored by krytarowski on Jul 18 2017, 5:22 AM.

Details

Summary

All 32 and 64 bit NetBSD platforms define off_t as 64-bit integer.

Part of the code inspired by the original work on libsanitizer in GCC 5.4 by Christos Zoulas.

Sponsored by <The NetBSD Foundation>

Diff Detail

Repository
rL LLVM

Event Timeline

krytarowski created this revision.Jul 18 2017, 5:22 AM
emaste added a subscriber: emaste.Jul 18 2017, 5:29 AM
emaste added inline comments.
lib/sanitizer_common/sanitizer_internal_defs.h
131–132

This comment needs to be updated or removed.

joerg edited edge metadata.Jul 18 2017, 6:00 AM

This doesn't make sense to me, both the existing and new code. off_t is a signed type.

Update comment

krytarowski retitled this revision from Define OFF_T as 64-bit unsigned integer on NetBSD to Define OFF_T as 64-bit integer on NetBSD.Jul 19 2017, 8:19 AM
krytarowski edited the summary of this revision. (Show Details)
krytarowski added inline comments.
lib/sanitizer_common/sanitizer_internal_defs.h
131–132

I've updated it and removed details. I assume that people are well aware about off64_t on Linux.

filcab added inline comments.Jul 19 2017, 9:40 PM
lib/sanitizer_common/sanitizer_internal_defs.h
135

I don't think rewording the comment should be done in the same commit as adding NetBSD support.

krytarowski added inline comments.Jul 20 2017, 6:35 AM
lib/sanitizer_common/sanitizer_internal_defs.h
135

I can add a note that NetBSD is special too. Is this fine?

filcab edited edge metadata.Jul 20 2017, 9:11 AM

Thank you,
Filipe

filcab added inline comments.Jul 20 2017, 10:24 AM
lib/sanitizer_common/sanitizer_internal_defs.h
135

Forgot to comment: Yes, it's fine. Feel free to reflow the comment if needed after your addition. If it's directly related to the patch, it makes sense to change the comment.

Update comment

emaste added inline comments.Aug 4 2017, 7:30 AM
lib/sanitizer_common/sanitizer_internal_defs.h
131–132

@filcab the list of OSes in the comment exactly repeats the following #if and adds no value, is there any reason we should repeat them in the comment?

vitalybuka accepted this revision.Aug 7 2017, 6:16 PM

Please fix the comment

This revision is now accepted and ready to land.Aug 7 2017, 6:16 PM

Please fix the comment

What's the desired comment? I will improve it on demand after committing this change.

krytarowski closed this revision.Aug 8 2017, 4:41 AM
vitalybuka added inline comments.Aug 8 2017, 11:35 AM
lib/sanitizer_common/sanitizer_internal_defs.h
131–132

about redundant oses list