Hi!
On Thu, 2014-08-21 at 21:12:20 +0200, Michael Vogt wrote:
> Package: debsig-verify
> Version: 0.10
> I ran into a issue today that debsig-verify would fail if $HOME was
> not writable to the debsig-verify progress. The reason is that gpg
> tries to create/read a ~/.gnupg/{pubring,secring}.gpg.
>
> Attached is a patch that run gpg with its own GNUPGHOME instead of the
> users.
Ah, makes sense, given that the gpg invoked is not using any default
options nor default keyrings. It should also have a more predictable
behavior. Thanks for the patch!
> diff -Nru debsig-verify-0.10/gpg-parse.c debsig-verify-0.10ubuntu1/gpg-parse.c
> --- debsig-verify-0.10/gpg-parse.c 2014-06-07 22:17:34.000000000 +0200
> +++ debsig-verify-0.10ubuntu1/gpg-parse.c 2014-08-21 20:59:04.000000000
> +0200
> @@ -32,16 +32,28 @@
> #include "debsig.h"
>
> static int gpg_inited = 0;
> +static char gpg_tmpdir[256] = {0,};
No need to initialize static variables, in any case see below.
> -/* Crazy damn hack to make sure gpg has created ~/.gnupg, else it will
> - * fail first time called */
> +/* Crazy damn hack to make sure gpg has a writable HOME to put its
> + trustdb and secret keyring etc */
I don't think the new behavior is crazy at all anymore, :) so the
comment would need to be updated.
> +static void cleanup_gpg_tmpdir(void) {
> + execl("/bin/rm", "rm", "-rf", gpg_tmpdir, NULL);
> +}
Please do not hardcode pathnames, use execlp() instead (I know the
existing codebase does not follow that, though :).
For new functions, or when changing function signatures, please use
the form:
,---
type
func_name(type)
{
body
}
`--
I'll try to add a coding-style file, probably referencing the one in
dpkg.
> static void gpg_init(void) {
> int rc;
>
> - if (gpg_inited) return;
> - rc = system(GPG_PROG" --options /dev/null < /dev/null > /dev/null 2>&1");
> - if (rc < 0)
> - ds_fail_printf(DS_FAIL_INTERNAL, "error writing initializing gpg");
> + if (gpg_inited)
> + return;
This conditional was not changed, so leave it as it was.
> + char *tmpdir = getenv("TMPDIR");
> + if(!tmpdir)
> + tmpdir = "/tmp";
> + snprintf(gpg_tmpdir, sizeof(gpg_tmpdir) -1,
> + "%s/%s", tmpdir, "debsig-verify.XXXXXX");
Use dpkg's path_make_temp_template() instead.
> + if(!mkdtemp(gpg_tmpdir))
> + ds_fail_printf(DS_FAIL_INTERNAL, "mkdtemp() failed for '%s'",
> gpg_tmpdir);
Use rc here instead of having function calls inside conditionals
(this also fixes rc not being used anymore and possible a warning).
And please spell the error message in a more user understandable form,
like "cannot create directory" or similar.
> + setenv("GNUPGHOME", gpg_tmpdir, 1);
> + atexit(cleanup_gpg_tmpdir);
You should check the return values for both calls.
> gpg_inited = 1;
> }
Thanks,
Guillem
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
