Re: [gmime-devel] [PATCH 2/6] Default to using "gpg"

[Daniel Kahn Gillmor]

On Fri 2016-12-02 09:19:29 -0500, Jeffrey Stedfast wrote:
> I've commited this patch with a slight modification...

thanks!  a bit more commentary interleaved below:

>> diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
>> index 0a05ed2..2ca280a 100644
>> --- a/gmime/gmime-gpg-context.c
>> +++ b/gmime/gmime-gpg-context.c
>> @@ -763,7 +763,7 @@ gpg_ctx_op_start (struct _GpgCtx *gpg)
>>  }
>>  
>>  /* run gpg */
>> -execvp (gpg->ctx->path, argv);
>> +execvp (gpg->ctx->path ? gpg->ctx->path : "gpg", argv);
>
> If g_mime_gpg_context_new () always sets path, then we don't need to do 
> this messy hack.

In other software projects, i've found it's useful to distinguish
between the "default" case and the case where something has been
explicitly set by the programmer.  For example, the config settings in
firefox's about:config for boolean values actually end up being a
tri-state: Default, User-set:True, and User-set:False.  This distinction
allows an update to firefox to change the defaults while not disturbing
people who've explicitly stated a commitment to one setting, even if it
happens to be the current default.

I admit that i don't have a specific case where that's useful in gmime
here (certainly we're not talking about the firefox code upgrade path
for an object that lives in memory), but preserving ctx->path == NULL
for the "default" case would provide a way for any code that wants to
find out whether this is "default" or not to know.

at any rate, i'm happy with either approach, just wanted to explain why
i don't think the above is actually a "messy hack" any more than the
other approach :)

Regards,

--dkg


signature.asc
Description: PGP signature
___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list

[gmime-devel] [PATCH 2/6] Default to using "gpg"

[Daniel Kahn Gillmor]

There is no reason to require a full path to the gpg executable.  We
should be able to leave it as the default.
---
 gmime/gmime-gpg-context.c | 10 +-
 tests/test-pgp.c  |  4 ++--
 tests/test-pgpmime.c  |  4 ++--
 tests/test-smime.c|  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gmime/gmime-gpg-context.c b/gmime/gmime-gpg-context.c
index 0a05ed2..2ca280a 100644
--- a/gmime/gmime-gpg-context.c
+++ b/gmime/gmime-gpg-context.c
@@ -763,7 +763,7 @@ gpg_ctx_op_start (struct _GpgCtx *gpg)
}

/* run gpg */
-   execvp (gpg->ctx->path, argv);
+   execvp (gpg->ctx->path ? gpg->ctx->path : "gpg", argv);
_exit (255);
} else if (gpg->pid < 0) {
g_strfreev (strv);
@@ -2190,7 +2190,7 @@ gpg_export_keys (GMimeCryptoContext *context, GPtrArray 
*keys, GMimeStream *ostr
 /**
  * g_mime_gpg_context_new:
  * @request_passwd: a #GMimePasswordRequestFunc
- * @path: path to gpg binary
+ * @path: path to gpg binary, or NULL for the default
  *
  * Creates a new gpg crypto context object.
  *
@@ -2203,10 +2203,10 @@ g_mime_gpg_context_new (GMimePasswordRequestFunc 
request_passwd, const char *pat
GMimeCryptoContext *crypto;
GMimeGpgContext *ctx;

-   g_return_val_if_fail (path != NULL, NULL);
-   
ctx = g_object_newv (GMIME_TYPE_GPG_CONTEXT, 0, NULL);
-   ctx->path = g_strdup (path);
+   if (path) {
+   ctx->path = g_strdup (path);
+   }

crypto = (GMimeCryptoContext *) ctx;
crypto->request_passwd = request_passwd;
diff --git a/tests/test-pgp.c b/tests/test-pgp.c
index bfdae74..3e7a2a4 100644
--- a/tests/test-pgp.c
+++ b/tests/test-pgp.c
@@ -289,7 +289,7 @@ int main (int argc, char **argv)
if (system ("/bin/mkdir ./tmp") != 0)
return EXIT_FAILURE;
setenv ("GNUPGHOME", "./tmp/.gnupg", 1);
-   if (system ("/usr/bin/gpg --list-keys > /dev/null 2>&1") != 0)
+   if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
return EXIT_FAILURE;

for (i = 1; i < argc; i++) {
@@ -304,7 +304,7 @@ int main (int argc, char **argv)

testsuite_start ("GnuPG crypto context");

-   ctx = g_mime_gpg_context_new (request_passwd, "/usr/bin/gpg");
+   ctx = g_mime_gpg_context_new (request_passwd, NULL);
g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) ctx, TRUE);

testsuite_check ("GMimeGpgContext::import");
diff --git a/tests/test-pgpmime.c b/tests/test-pgpmime.c
index 546df1b..b5a8b21 100644
--- a/tests/test-pgpmime.c
+++ b/tests/test-pgpmime.c
@@ -425,7 +425,7 @@ int main (int argc, char *argv[])
if (system ("/bin/mkdir ./tmp") != 0)
return EXIT_FAILURE;
setenv ("GNUPGHOME", "./tmp/.gnupg", 1);
-   if (system ("/usr/bin/gpg --list-keys > /dev/null 2>&1") != 0)
+   if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
return EXIT_FAILURE;

for (i = 1; i < argc; i++) {
@@ -440,7 +440,7 @@ int main (int argc, char *argv[])

testsuite_start ("PGP/MIME implementation");

-   ctx = g_mime_gpg_context_new (request_passwd, "/usr/bin/gpg");
+   ctx = g_mime_gpg_context_new (request_passwd, NULL);
g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) ctx, TRUE);

testsuite_check ("GMimeGpgContext::import");
diff --git a/tests/test-smime.c b/tests/test-smime.c
index f4549c7..f05e03a 100644
--- a/tests/test-smime.c
+++ b/tests/test-smime.c
@@ -426,7 +426,7 @@ int main (int argc, char *argv[])
if (system ("/bin/mkdir ./tmp") != 0)
return EXIT_FAILURE;
g_setenv ("GNUPGHOME", "./tmp/.gnupg", 1);
-   if (system ("/usr/bin/gpg --list-keys > /dev/null 2>&1") != 0)
+   if (system ("gpg --list-keys > /dev/null 2>&1") != 0)
return EXIT_FAILURE;

for (i = 1; i < argc; i++) {
-- 
2.10.2

___
gmime-devel-list mailing list
gmime-devel-list@gnome.org
https://mail.gnome.org/mailman/listinfo/gmime-devel-list