Page MenuHomePhabricator

Centos 5 compile fixes for lldb
Needs RevisionPublic

Authored by mchandler-blizzard on Oct 29 2015, 12:11 PM.

Details

Reviewers
ovyalov
Summary

Adds checks for posix features not present in centos 5

Diff Detail

Repository
rL LLVM

Event Timeline

mchandler-blizzard retitled this revision from to Centos 5 compile fixes for lldb.
mchandler-blizzard updated this object.
mchandler-blizzard added reviewers: labath, ovyalov.
mchandler-blizzard set the repository for this revision to rL LLVM.
mchandler-blizzard added a subscriber: lldb-commits.
labath requested changes to this revision.Oct 29 2015, 5:46 PM
labath edited edge metadata.

Thanks for the patch. Looks good, but we need to make sure things continue to work on non-linux systems...

tools/lldb/source/Host/common/File.cpp
301

This needs to evaluate to true on non-linux systems.
Would #ifdef O_CLOEXEC work for you ?

This revision now requires changes to proceed.Oct 29 2015, 5:46 PM
krytarowski added inline comments.
tools/lldb/source/Host/common/File.cpp
301

NetBSD:

/usr/include/fcntl.h:#define	O_CLOEXEC	0x00400000	/* set close on exec */
krytarowski requested changes to this revision.Oct 29 2015, 5:56 PM
krytarowski added a reviewer: krytarowski.
krytarowski added inline comments.
tools/lldb/source/Host/posix/PipePosix.cpp
32

Is this file just for Linux?

254

Stop breaking NetBSD.

brucem requested changes to this revision.Oct 29 2015, 6:01 PM
brucem added a reviewer: brucem.
brucem added a subscriber: brucem.
brucem added inline comments.
tools/lldb/source/Host/linux/HostThreadLinux.cpp
33

This is probably better done with weak symbols or a check in configure and cmake rather than hard-coding this stuff in the C pre-processor.

tools/lldb/source/Host/posix/PipePosix.cpp
32

This file isn't Linux only, so the concerns expressed by Labath are valid here as well. (The same thing is true of other inclusions of <linux/version.h>.)

tools/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
72

This bug was fixed in 2007, over 8 years ago ... at what point do we stop carrying around this baggage?

krytarowski added inline comments.Oct 29 2015, 6:10 PM
tools/lldb/source/Host/posix/PipePosix.cpp
43

How about moving PIPE2_SUPPORTED to include/lldb/Host/*/Config.h? It could be renamed to LLDB_CONFIG_PIPE2_SUPPORTED. Feel free to include in the Linux configuration headers needed to detect Linux version.

krytarowski added inline comments.Oct 29 2015, 7:25 PM
tools/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
72

Few defines aren't much of entanglement. RHEL5 is still supported upstream (till 2017).

https://en.wikipedia.org/wiki/Red_Hat_Enterprise_Linux#Product_life_cycle

tools/lldb/source/Host/common/File.cpp
301

If that is ok by the coding standards im happy to change it to that

tools/lldb/source/Host/linux/HostThreadLinux.cpp
33

If you want to provide code that does that, im happy to change it.

tools/lldb/source/Host/posix/PipePosix.cpp
32

I dont have other OS's to build on so not sure. Think @krytarowski comment below is the way to go.

254

:(

emaste added a subscriber: emaste.Oct 30 2015, 2:20 PM

For future diffs please include additional context (e.g. git diff -U99999)

labath resigned from this revision.Mar 15 2016, 1:38 AM
labath removed a reviewer: labath.
krytarowski resigned from this revision.May 12 2016, 1:46 PM
krytarowski removed a reviewer: krytarowski.

This patch looks dead -- no activity.

brucem removed a reviewer: brucem.Jul 19 2017, 3:09 AM