ci: Bump GCC_SNAPSHOT_MAJOR to 15 #1583

1. 
   maflcko commented at 5:42 pm on August 12, 2024: contributor

   Follow-up to #1313

   Clang should silently follow the main devel branch, but GCC needs to be bumped manually.

2. ci: Bump GCC_SNAPSHOT_MAJOR to 15 e34b476730
3. 
   real-or-random commented at 6:50 pm on August 12, 2024: contributor

   GCC 15 has a new warning, see https://gcc.gnu.org/pipermail/gcc-patches/2024-June/656014.html .
4. 
   maflcko commented at 7:36 pm on August 12, 2024: contributor

   Yeah, I guess one solution would be to be verbose and write the array:

   0static const unsigned char PREFIX[19] = {'s', 'e', 'c', 'p', '2', '5', '6', 'k', '1', ' ', 't', 'e', 's', 't', ' ', 'i', 'n', 'i', 't'};
   

   an alternative would be to disable the warning, which seems cleaner and any issues that the warning theoretically could uncover should be detectable by a the existing CI sanitizer builds?

5. 
   real-or-random commented at 9:48 pm on August 12, 2024: contributor

   0static const unsigned char PREFIX[19] = {'s', 'e', 'c', 'p', '2', '5', '6', 'k', '1', ' ', 't', 'e', 's', 't', ' ', 'i', 'n', 'i', 't'};
   

   This is ultimately a question of taste, but I suggest this because I think it’s the most readable:

   0static const unsigned char PREFIX[] = {'s', 'e', 'c', 'p', '2', '5', '6', 'k', '1', ' ', 't', 'e', 's', 't', ' ', 'i', 'n', 'i', 't'};
   

   This makes it unambiguously clear that we consider it an array and not a string (i.e., it’s not null-terminated), the reader doesn’t need to count the chars, and we can use sizeof(PREFIX).

   We define such constant arrays in multiple code locations, see https://github.com/bitcoin-core/secp256k1/actions/runs/10356727938/job/28667982412?pr=1583 Whenever I review such code, I need to count the chars manually to check whether the terminating null byte is included or not.

6. refactor: Use array initialization for unterminated strings

   The previous code is correct and harmless to initialize an array with a
   non-terminated character sequence using a string literal.
   
   However, it requires exactly specifying the array size, which can be
   cumbersome.
   
   Also, GCC-15 may issue the -Wunterminated-string-initialization warning.
   [1]
   
   Fix both issues by using array initialization. This refactoring commit
   does not change behavior.
   
   [1] Example warning:
   
   src/modules/schnorrsig/main_impl.h:48:46: error: initializer-string for array of 'unsigned char' is too long [-Werror=unterminated-string-initialization]
      48 | static const unsigned char bip340_algo[13] = "BIP0340/nonce";
         |                                              ^~~~~~~~~~~~~~~

   fa67b6752d
7. maflcko force-pushed on Aug 15, 2024
8. 
   maflcko commented at 5:03 pm on August 15, 2024: contributor

   Whenever I review such code, I need to count the chars manually to check whether the terminating null byte is included or not.

   Good point about the review overhead and ambiguity. I pushed a commit with your suggestion.

9. hebasto approved
10. 
    hebasto commented at 7:19 pm on August 15, 2024: member

    ACK fa67b6752d8ba3e4c41f6c36b1c6b94a21770419, I have reviewed the code and it looks OK.

    I can see gcc version 15.0.0 20240811 (experimental) (GCC) in the CI logs.

11. real-or-random approved
12. 
    real-or-random commented at 2:04 pm on August 17, 2024: contributor

    utACK fa67b6752d8ba3e4c41f6c36b1c6b94a21770419
13. real-or-random added the label ci on Aug 17, 2024
14. real-or-random added the label refactor/smell on Aug 17, 2024
15. real-or-random added the label assurance on Aug 17, 2024
16. real-or-random merged this on Aug 17, 2024
17. real-or-random closed this on Aug 17, 2024

    ---

18. maflcko deleted the branch on Aug 19, 2024

Contributors
maflcko real-or-random hebasto

Labels
assurance ci refactor/smell