Simon Josefsson simon@josefsson.org writes:
- Is including HTML and PDF manuals in 'make dist' tarballs still
useful? Reproducing the tarballs from git requires more tools, especially the PDF. Removing them reduces tarball from 2.7MB to 2.0MB.
I'm not sure which formats people find useful. I think it's valuable that one can get the tarball and read the docs without having to build or generate anything.
Nettle releases are a bit old-fashioned, with little thought to reproducibility and transparency.
- The tarball embeds some username info, some
portability/reproducability TAR_OPTIONS for inspiration:
export TAR_OPTIONS = --owner=0 --group=0 --numeric-owner --sort=name --mode=go+u,go-w --format=ustar
Trimming the tar meta data a little seems like an easy improvement. When I looked at this in a different project, I ended up with the invocation here: https://git.glasklar.is/system-transparency/core/system-transparency/-/blob/...
tar --exclude .git --exclude .gitmodules --sort=name --format=posix \ --pax-option=exthdr.name=%d/PaxHeaders/%f \ --pax-option=delete=atime,delete=ctime \ --clamp-mtime --mtime="./$(basename "${DIST_DIR}")/${LATEST_COMPONENT}" \ --numeric-owner --owner=0 --group=0 \ --mode=go+u,go-w \ -cf - "$(basename "${DIST_DIR}")" ) | gzip --no-name --best > "${DIST_DIR}.tar.gz"
based mostly on the section on reproducibility in the GNU tar manual. Is there some reason to prefer format ustar over posix?
If I remember correctly, fiddling with file timestamps was also rather important to get a reproducible tar file. But it may be a good first step to fix the non-timestamp metadata?
- On a riscv64 machine (RevyOS, derived from Debian trixie, with gcc
--version 'gcc (Debian 14.3.0-10revyos2) 14.3.0') I got the warning below, and I'm not sure why it doesn't show up on amd64. Self-tests passes fine on the platform.
In function ‘xalloc’, inlined from ‘test_aead_message’ at testutils.c:989:19: testutils.c:37:13: warning: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=] 37 | void *p = malloc(size); | ^~~~~~~~~~~~ In file included from testutils.h:11, from testutils.c:3: /usr/include/stdlib.h: In function ‘test_aead_message’: /usr/include/stdlib.h:672:14: note: in a call to allocation function ‘malloc’ declared here 672 | extern void *malloc (size_t __size) __THROW __attribute_malloc__ | ^~~~~~
I've seen similar warnings occationally on other platforms, without making any sense of it. And it appears the code looks like
uint8_t *buf = xalloc (cipher->length + 1); uint8_t *copy = xalloc (cipher->length);
where your compiler warns for the second line, but not the first.
- Enabling -fanalyzer shows the warning below. I've had mixed
experiences with -fanalyzer, but I've had real positives with it.
Would be interesting to get gcc's analyzer to run reliably in the CI (currently, it runs clang's static analyzer). But I can't make sense of this report. As I read it, it is the code path that processes complete blocks.
| 184 | if ((ctx)->index) \ | | ^ | | | | | (2) following ‘false’ branch...
Means that the block buffer is empty, and
| 205 | while ((length) >= sizeof((ctx)->block)) \ | | ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~ | | | | | (4) following ‘true’ branch (when ‘length > 31’)...
means that caller's input is a complete block (or more). That input block is eventually passed to gost_compute_sum_and_hash, which reads it as 8 little-endian 32-bit numbers
uint32_t block_le[8]; ... for (i = carry = 0; i < 8; i++, block += 4) { block_le[i] = LE_READ_UINT32(block); ...
and passes to gost_block_compress which uses this to initizes a working copy,
uint32_t key[8], u[8], v[8], w[8], s[8]; ... memcpy (v, block, sizeof (v));
At this point, all of v should be properly initialized, unless the external caller passes an uninitialized message.
The trace from the analyzer mention this memcpy call as a relevant event, maybe that is where it loses track? Analyzer says
| 69 | uint32_t key[8], u[8], v[8], w[8], s[8]; | | ~ | | | | | (14) region created on stack here |...... | 76 | w[0] = u[0] ^ v[0], w[1] = u[1] ^ v[1]; | | ~~~~ | | | | | (15) use of uninitialized value ‘v[1]’ here
- Another warning, just for testsuite:
poly1305-test.c: In function ‘test_fixed’: poly1305-test.c:184:7: error: ‘test_poly1305_internal’ reading 16 bytes from a region of size 1 [-Werror=stringop-overread] 184 | test_poly1305_internal (key->data, message->length, message->data, nonce, digest->data); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I think this is related to the traditional "flexible arrays" used by the test type
struct tstring { struct tstring *next; size_t length; uint8_t data[1]; };
If I look at https://en.cppreference.com/w/c/language/array.html, it seems that instead declaring the flexible member as
uint8_t data[];
should be allowed in c99, so it's doable to update to the more modern style.
Regards, /Niels