[PATCH] Add inside-Emacs mode to GUI pinentry programs
ueno at gnu.org
Wed Jun 17 03:32:22 CEST 2015
Thanks for the review, and sorry for late response. I was a bit
occupied with other tasks.
"Neal H. Walfield" <neal at walfield.org> writes:
> At Sun, 14 Jun 2015 10:24:04 +0900,
> Daiki Ueno wrote:
>> - the UID value check is now replaced with simple cast to uint32_t,
>> using configure time check
> Is this really safe? Before you caught the error, now we silently
> take the value mod 2^32.
Right, I added a check: uid == (uint32_t) uid.
The intention of the above change was to suppress a compiler warning
saying that a check like uid <= ULONG_MAX always evaluates to true.
>> However, perhaps the above scenario might be actually possible by
>> explicitly unsetenv'ing INSIDE_EMACS in gpg-agent, when
>> allow-emacs-pinentry is not set in gpg-agent.conf (like we do for
>> DISPLAY, when keep-display is not set).
> I seem to remember Werner suggesting that this would require a large
> change, but if it is easy, I would welcome this approach.
I leave this as is for now. I think we could remove the Assuan option
later if restricting the envvar is sufficient for our purpose.
>> pinentry_cmd_handler_t fallback_cmd_handler;
> Is there any reason this isn't marked static?
> If set_socket is successful, but read_from_emacs fails, then
> emacs_socket will be valid (>= 0) and the test in
> initial_emacs_cmd_handler will fail. I think
> initial_emacs_cmd_handler either needs to test the return result of
> pinentry_emacs_init or pinentry_emacs_init needs to deinit the socket
> if emacs doesn't have pinentry support.
I took the latter approach and added a code to deinit emacs_socket in
other appropriate places as well.
> pinentry_enable_emacs_cmd_handler can be called multiple times, but
> this function isn't designed for this.
> pinentry_enable_emacs_cmd_handler (void)
> const char *envvar;
Good point. I added a check if pinentry_cmd_handler is already set to
initial_emacs_cmd_handler or emacs_cmd_handler.
> Also, since pinentry_enable_emacs_cmd_handler is only called from the
> option handler, I don't think we need the field allow_emacs_prompt in
> the pinentry structure.
Yes, removed the field.
Other changes from the previous version are:
- Removed allocation code from read_from_emacs and simplified the
- Fixed minor errors detected by clang-analyzer
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 27367 bytes
Desc: not available
More information about the Gnupg-devel