Bugfix: Define and use HAVE_FDATASYNC correctly outside LevelDB #19803

1. 
   luke-jr commented at 4:51 pm on August 25, 2020: member

   Fixes a bug introduced in #19614

   The LevelDB-specific fdatasync check was only using AC_SUBST, which works for Makefiles, but doesn’t define anything for C++. Furthermore, the #define is typically 0 or 1, never undefined.

   This fixes both issues by defining it and checking its value instead of whether it is merely defined.

   Pulled out of #14501 by fanquake’s request

2. Bugfix: Define and use HAVE_FDATASYNC correctly outside LevelDB c4b85ba704
3. DrahtBot added the label Build system on Aug 25, 2020
4. DrahtBot added the label Utils/log/libs on Aug 25, 2020
5. 
   DrahtBot commented at 1:13 am on August 26, 2020: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #19430 (build: configure.ac and netbsd-build.md for NetBSD by midnightmagic)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

6. 
   fanquake commented at 1:50 pm on August 27, 2020: member

   Thanks for catching, and splitting this out. I’m sure I checked that we dropped into the #if defined(HAVE_FDATASYNC) before PR’ing #19614, but clearly I didn’t test well enough.
7. 
   laanwj commented at 1:51 pm on August 28, 2020: member

   Whoa. Thanks for paying attention here. Code review ACK c4b85ba704a1b5257dc82786a84f676bacb7b027 I’ve checked it now gets defined and used correctly. Clearly #if … is safer here because at least it’ll raise an error if it ceases being defined for some reason.
8. fanquake approved
9. 
   fanquake commented at 4:56 am on August 31, 2020: member

   ACK c4b85ba704a1b5257dc82786a84f676bacb7b027 - thanks for catching and fixing my mistake.

   Clearly #if … is safer here because at least it’ll raise an error

   It’ll also warn when not defined if you pass -Wundef,

   0util/system.cpp:1022:9: warning: "HAVE_FDATASYNC" is not defined, evaluates to 0 [-Wundef]
   1     #if HAVE_FDATASYNC
   2         ^~~~~~~~~~~~~~
   

   which would have made this more obvious. I had looked at turning this on before, as it has caught other problems (#18359), but there were issues with dependencies. I’m going to follow up with up #16419.

10. fanquake merged this on Aug 31, 2020
11. fanquake closed this on Aug 31, 2020

    ---

12. sidhujag referenced this in commit b58c80dbee on Aug 31, 2020
13. laanwj referenced this in commit 2e8116149c on Feb 23, 2021
14. sidhujag referenced this in commit 0939a207f6 on Feb 24, 2021
15. DrahtBot locked this on Feb 15, 2022

Contributors
luke-jr DrahtBot fanquake laanwj

Labels
Build system Utils/log/libs