Simon Josefsson <[email protected]> writes: > 1) 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. > 2) 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/main/releng/mk-release-archive.sh?ref_type=heads#L197 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? > 3) 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. > 4) 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 > 5) 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 -- Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677. Internet email is subject to wholesale government surveillance. _______________________________________________ nettle-bugs mailing list -- [email protected] To unsubscribe send an email to [email protected]
