This is an archive of the discontinued LLVM Phabricator instance.

Eliminate custom typedefs in the FreeBSD case and include sys/types.h to define standard types. This is currently fixing a build failure on x86_64 FreeBSD.
ClosedPublic

Authored by seanbruno on Nov 30 2015, 12:35 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

seanbruno updated this revision to Diff 41426.Nov 30 2015, 12:35 PM
seanbruno retitled this revision from to Eliminate custom typedefs in the FreeBSD case and include sys/types.h to define standard types. This is currently fixing a build failure on x86_64 FreeBSD..
seanbruno updated this object.
seanbruno added a reviewer: davidxl.
davidxl accepted this revision.Nov 30 2015, 12:37 PM
davidxl edited edge metadata.

LGTM assuming it works as expected.

This revision is now accepted and ready to land.Nov 30 2015, 12:37 PM

Any blocker for this to be committed ?

Any blocker for this to be committed ?

No that I'm aware of.

Re-examine the patch a little bit. Have you checked older FreeBSD system works with the patch? See the old comment in the code:

System headers define 'size_t' incorrectly on x64 FreeBSD (prior to

  • FreeBSD 10, r232261) when compiled in 32-bit mode

PRIu64 macro also does not seem to be defined in sys/types.h but in inttypes.h

emaste added a subscriber: emaste.Dec 18 2015, 2:23 PM
davidxl edited edge metadata.Dec 30 2015, 9:22 AM
davidxl added subscribers: brooks, dim.
dim added a comment.Dec 30 2015, 10:37 AM

Re-examine the patch a little bit. Have you checked older FreeBSD system works with the patch? See the old comment in the code:

System headers define 'size_t' incorrectly on x64 FreeBSD (prior to

  • FreeBSD 10, r232261) when compiled in 32-bit mode

If I recall correctly, FreeBSD didn't properly support compiling in 32-bit mode on a 64-bit x86 platform anyway, before this commit. So maybe the issue is a little moot now. Also, no supported releases of FreeBSD have the old-style headers anymore.

On the other hand, we could also test __FreeBSD_version here. @emaste, what do you think about it? I believe >= 1000010 and and >= 902509 are the versions where the header is guaranteed to be correct.

PRIu64 macro also does not seem to be defined in sys/types.h but in inttypes.h

Yes, it is indeed defined there. Let's just include inttypes.h, then.

Ok thanks. I will first commit a patch that includes inttypes.h and
sys/types.h for FreeBSD. If there is a need to support older
versionsof FreeBSD, we can add checks for FreeBSE version.

David

This revision was automatically updated to reflect the committed changes.