GNOME Bugzilla – Bug 750366
Test fixes for 2015.7 and 32 bit
Last modified: 2015-06-05 21:34:21 UTC
Here are some fixes found after testing 2015.7 on 32 bit.
Created attachment 304551 [details] [review] Fix tests on 32 bit systems Use guint64 when the 't' format is used for GVariant
Created attachment 304552 [details] [review] Fix double free in ostree_repo_pull_with_options `contents` is always inserted as a value into `requested_refs_to_fetch`, which is initialized to `g_free` its values when the function returns. `contents` is also sometimes inserted as a key into `expected_commit_sizes`, which previously was also set to `g_free` its keys. This meant that in this case `contents` would be freed twice. By making `expected_commit_sizes` not free its keys, `contents` is freed exactly once, and since this is the only place `expected_commit_sizes` is modified this change does not introduce any leaks elsewhere.
Created attachment 304553 [details] [review] tests: Link test-gpg-verify-result with gpgme This test uses gpgme directly to verify the signatures, so it needs to find the gpgme headers and link with gpgme to ensure the linker can resolve the symbols.
Created attachment 304554 [details] [review] tests: Use readdir64 when _FILE_OFFSET_BITS set On 32 bit systems, _FILE_OFFSET_BITS will be set to 64 by AC_SYS_LARGEFILE. This causes the glibc headers to use readdir64 rather than readdir. Emulate that behavior in the preloader or the tests will all fail with "No such file or directory".
Created attachment 304555 [details] [review] tests: Use temporary gpg homedir libtest always makes a copy of the gpghome directory to the test directory, so there's no need to operate on the installed copy. This allows test-remote-gpg-import to pass as an unprivileged user since it otherwise couldn't create the temp files gpgme creates.
Created attachment 304556 [details] [review] Revert "tests: skip test-commit-sign.sh when not root" This reverts commit d3545b0661f3247cd8c106e64a71052ce9952243. Since the test is now using the temporary copy of the gpg homedir, it is no longer owned by root.
Review of attachment 304553 [details] [review]: LGTM
Review of attachment 304554 [details] [review]: Hum. But don't we need to also change the entrypoint symbol that we're preloading to be readdir64? Unless...oh I see, the C preprocessor does that for us. Except actually it's not the preprocessor, it's this __REDIRECT magic that associates an "asmname" with it. Hmm...I played around a bit trying to get __ASMNAME() to get us the right thing in a string form, but eh, this patch works.
Review of attachment 304552 [details] [review]: This patch looks correct, but I'd prefer one which simply did the strdup() so that the hash table ownership is independent. The cost of that is pretty small versus the programmer safety we get from any future refactoring.
Review of attachment 304552 [details] [review]: Makes sense. I'll rework it to do that.
Review of attachment 304554 [details] [review]: The __REDIRECT magic really threw me off. I thought I would need to add another readdir64 entry point, but then why was it trying to load the readdir symbol in the first place? I figured that the program would try to load readdir64 directly, but I guess that's not the case.
Review of attachment 304555 [details] [review]: Nice, thank you! That'd been bothering me a while.
Review of attachment 304556 [details] [review]: Yep.
Created attachment 304667 [details] [review] Fix double free in ostree_repo_pull_with_options Duplicate the commit checksum for expected_commit_sizes since it's also used as a value in requested_refs_to_fetch.
Attachment 304667 [details] pushed as 4f6f97c - Fix double free in ostree_repo_pull_with_options