-
Notifications
You must be signed in to change notification settings - Fork 8
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
Portability improvements, cleanup, and bug fixes #36
Portability improvements, cleanup, and bug fixes #36
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
ff2c6ee
to
5fc3404
Compare
This comment was marked as outdated.
This comment was marked as outdated.
6df18ea
to
f83521c
Compare
@larsbrinkhoff: Is this going to get integrated? |
@bscottm, I think so, but there was some discussion and I'm not sure everything was resolved. I'll take a look now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments about things that I'd like to see changed.
This comment was marked as resolved.
This comment was marked as resolved.
Maybe we should get this in as is, since it contains lots of good changes. And any other fixes can be done in a different PR? |
This comment was marked as outdated.
This comment was marked as outdated.
I think this is the wrong approach. "Fix later" often becomes "fix never". A pull request should be cleaned up before merging. If it seems to take too long, or is too much work, maybe open a new pull request with only the good commits. |
This comment was marked as resolved.
This comment was marked as resolved.
There is also the other side of the coin, that continuously fixing minor points here and there it will never get merged because the person doing the work runs out of steam. Many of the changes here are good ones, some are more of minute importance, and some are just why. Splitting it up sounds like a good idea. |
6f7860c
to
2c1c27e
Compare
* Add or fix support for building on Solaris, illumos, Linux/musl, NetBSD, OpenBSD, IBM AIX, IBM OS/400 (PASE), Cygwin, and Haiku, which weren't previously cleanly building. * Use `pkg-config` when available to determine CFLAGS and libraries. * Initialize some variables in `supdupd`. * Fix off-by-one error in `supdup`. * Suppress spurious error when Chaosnet bridge socket is missing. * Create `README.md`. * Cleanup Makefile. * Properly detect bad ports (negative or >65535). * Fix non-ANSI function declarations. * Add `*.ln` (*Lint*) files, `supdupd`, and `compile_commands.json` to `.gitignore`. * Create `.gitattributes`. * Ensure file handle is valid (non-negative) before closing. * Correct various spelling errors. * Tested build under Oracle Solaris, OpenIndiana illumos, Linux/musl, Linux/glibc, IBM AIX, IBM OS/400 (PASE), Haiku, FreeBSD, OpenBSD, NetBSD, DragonFly BSD, Cygwin, and Apple macOS. * Compilation tested and working with PCC, GCC, Clang, Xcode, IBM XL C, IBM Open XL C, Oracle Studio C, NVIDIA HPC SDK C, Portland Group C, and DMD ImportC. * Passes Cppcheck and Clang Analyzer. * Closes: PDP-10#25, PDP-10#35, PDP-10#38 Signed-off-by: Jeffrey H. Johnson <trnsz@pobox.com>
Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved.
pkg-config
when available to determine CFLAGS and libraries.supdupd
.supdup
.README.md
.*.ln
(Lint) files,supdupd
, andcompile_commands.json
to.gitignore
..gitattributes
.Tested build under Oracle Solaris, OpenIndiana illumos, Linux/musl, Linux/glibc, IBM AIX, IBM OS/400 (PASE), Haiku, FreeBSD, OpenBSD, NetBSD, DragonFly BSD, Cygwin, and Apple macOS.
Compilation tested and working with PCC, GCC, Clang, Xcode, IBM XL C, IBM Open XL C, Oracle Studio C, NVIDIA HPC SDK C, Portland Group C, and DMD ImportC.
Passes Cppcheck and Clang Analyzer.
Signed-off-by: Jeffrey H. Johnson <trnsz@pobox.com>