Conversation
[bitcoind] Make forking error messages consistent
|
Concept ACK |
|
Concept ACK |
| AC_CHECK_DECLS([strnlen]) | ||
|
|
||
| # Check for daemon(3), unrelated to --with-daemon (although used by it) | ||
| AC_CHECK_DECLS([daemon]) |
|
Concept ACK |
|
I'm not overly concerned with making the feature freeze but I realised it's a bad code smell to have two mutually exclusive command line arguments, so have simply removed the chdir for the time being. |
|
Thanks. Will test. |
|
I get the following compile error when I force HAVE_DECL_DAEMON off: Where is that definition supposed to come from? |
|
It's from /usr/include/paths.h but now that I see the initial _ (and, of course, your error) I wonder how portable it is. The file's there on OpenBSD and Linux. I don't have anything else to hand to look in but notably the file's copyright is 1993 to Berkely so if not officially portable it's probably practically portable. According to gnulib's manual: I'll push a change shortly to be more thorough. |
|
Anything holding this up? |
|
Yes, it's not compiling for me when pretending not to have daemonize(). If we can't test that fallback code, we shouldn't even include it. |
|
Note that we do have some things that potentially print to stdout/stderr after daemonize():
Should not hold up this pull, as this is bad behavior in the first place, but we should make sure that everything printed also ends up in the log. |
|
@ChoHag Around? If not then maybe someone else wants to pick this up? |
|
I'm not really, I have pressing personal matters to deal with and cannot Matthew
|
|
Are there any POSIX-ish OSes that are supported by our build that don't support If not, we could just disable -daemon if that call is not available (like on WIndows). That makes it a lot easier to do this as well as maintain the code afterward as the fallback path is not needed. |
|
Closing in favor of #8813 |
Simplified version of bitcoin#8278. Assumes that every OS that (a) is supported by Bitcoin Core (b) supports daemonization has the `daemon()` function in its C library. - Removes the fallback path for operating systems that support daemonization but not `daemon()`. This prevents never-exercised code from ending up in the repository (see discussion here: bitcoin#8278 (comment)). - Removes the windows-specific path. Windows doesn't support `daemon()`, so it don't support daemonization there, automatically. Original code by Matthew King, adapted by Wladimir van der Laan.
Simplified version of bitcoin#8278. Assumes that every OS that (a) is supported by Bitcoin Core (b) supports daemonization has the `daemon()` function in its C library. - Removes the fallback path for operating systems that support daemonization but not `daemon()`. This prevents never-exercised code from ending up in the repository (see discussion here: bitcoin#8278 (comment)). - Removes the windows-specific path. Windows doesn't support `daemon()`, so it don't support daemonization there, automatically. Original code by Matthew King, adapted by Wladimir van der Laan. Github-Pull: bitcoin#8813 Rebased-From: a92bf4a
When started with cron, the sendmail process hung around waiting for its stdin, bitcoind's stdout/err, to close. Fix that using daemon(3) where possible and doing its work where not.
Also makes the forking error messages more consistent and, by using strerror, more explanatory.