Define SECP256K1_BUILD in secp256k1.c directly. #928

1. 
   gmaxwell commented at 5:13 pm on May 1, 2021: contributor

   This avoids building without it and makes it safer to use a custom building environment. Test harnesses need to #include secp256k1.c first now.

   Fixes #927

2. 
   sipa commented at 5:39 pm on May 1, 2021: contributor

   Does this risk accidentally including secp256k1.h somewhere before secp256k1.c, thus getting the wrong interface definitions? If so, perhaps it’s a possibility to have .h define a SECP256K1_NO_BUILD (if SECP256K1_BUILD wasn’t defined already). .c can then error out if SECP256K1_NO_BUILD was defined.
3. 
   gmaxwell commented at 5:49 pm on May 1, 2021: contributor

   Indeed and the PR fixed a couple tests that were doing exactly that. I didn’t consider it much of a concern because the non-null warnings catch it in development… but you’re absolutely right that this is easy to avoid and so I added your suggestion.
4. in include/secp256k1.h:139 in 4849a7b250 outdated

   126@@ -127,6 +127,10 @@ typedef int (*secp256k1_nonce_function)(
   127 #  define SECP256K1_INLINE inline
   128 # endif
   129 
   130+#ifndef SECP256K1_BUILD
   131+# define SECP256K1_NO_BUILD
   132+#endif
   

   ---

   ---

   

   real-or-random commented at 6:21 pm on May 1, 2021:

   nit: I think this would be a good place to spend one or two sentences to explain the purpose of these macros. (My personal experience: I think I had two ask a few times before I understood the entire concept.)

   ---

   

   gmaxwell commented at 7:28 pm on May 1, 2021:

   There is a comment below on the nullness macro. But I can add one to the guard about the guard specifically.
5. 
   real-or-random commented at 6:21 pm on May 1, 2021: contributor

   ACK mod nit
6. Define SECP256K1_BUILD in secp256k1.c directly.

   This avoids building without it and makes it safer to use a custom
    building environment.  Test harnesses need to #include secp256k1.c
    first now.

   ae9e648526
7. 
   sipa commented at 7:58 pm on May 1, 2021: contributor

   utACK ae9e648526ceaf7cd97ba4dfe3c105db8e226c35
8. real-or-random approved
9. 
   real-or-random commented at 9:38 am on May 2, 2021: contributor

   ACK ae9e648526ceaf7cd97ba4dfe3c105db8e226c35
10. real-or-random merged this on May 2, 2021
11. real-or-random closed this on May 2, 2021

    ---

12. real-or-random cross-referenced this on May 2, 2021 from issue changed include statements without prefix 'include/' by whb07
13. real-or-random cross-referenced this on May 2, 2021 from issue Tracking issue for manual (non-autotools) builds by real-or-random
14. jonasnick cross-referenced this on Jun 14, 2021 from issue Upstream PRs 831, 907, 903, 889, 918, 906, 928, 922, 933, Merge bitcoin-core/secp256k1#936: Fix gen_context/ASM build on ARM, 925, 937, 926, Merge bitcoin-core/secp256k1#940: contrib: Explain explicit header guards, 850, 930, 941, 846, 947, 662, 950 by jonasnick
15. in src/secp256k1.c:7 in ae9e648526

    3@@ -4,6 +4,8 @@
    4  * file COPYING or https://www.opensource.org/licenses/mit-license.php.*
    5  ***********************************************************************/
    6 
    7+#define SECP256K1_BUILD
    

    ---

    ---

    

    elichai commented at 12:31 pm on June 30, 2021:

    Shouldn’t this be wrapped in #ifndef SECP256k1_BUILD?

    ---

    

    gmaxwell commented at 6:48 pm on June 30, 2021:

    I don’t see why– if it was you wouldn’t have spotted that your rust build was incorrectly setting it now that it shouldn’t be set that way anymore.

    ---

    

    elichai commented at 8:38 pm on June 30, 2021:

    I just thought it was common practice to wrap defines in ifndef in the case that the define acts as a flag and doesn’t carry any additional value. (I don’t think there was any harm in the fact that we kept passing -DSECP256K1_BUILD to the compiler, as we don’t really use “headers” in rust)
16. elichai cross-referenced this on Jun 30, 2021 from issue Fix a C compiler warning because of redefinition of SECP256K1_BUILD by elichai
17. real-or-random cross-referenced this on Jul 6, 2022 from issue SECP256K1_BUILD sanity checking and USE_FIELD_INV_BUILTIN/USE_SCALAR_INV_BUILTIN simplification. by gmaxwell

Contributors
gmaxwell sipa real-or-random elichai