[PATCH] Add inside-Emacs mode to GUI pinentry programs

[attila]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA384

Hello gnupg-devel, Neal and Daiki,

I've been following the discussion with some interest and only wish to
make one slight comment:

Neal H. Walfield <neal at walfield.org> writes:

> Hi Daiki,
>
> [snip]
>> +  if (strlen (tmpdir) + strlen (socket_name) + 10 + 2
>> +      >= sizeof (emacs_server_address.sun_path))
>> +    {
>> +      fprintf (stderr, "socket name is too long\n");
>> +      free (tmpdir_storage);
>> +      return 0;
>> +    }
>> +
>> +  sprintf (emacs_server_address.sun_path, "%s/emacs%lu/%s",
>> +	   tmpdir, (unsigned long) uid, socket_name);
>
> I think the check above is wrong.  It should be something like:
>
>   if (strlen (tmpdir) + strlen("/emacs") + 10 + strlen("/")
>       + strlen (socket_name) + 1
>       >= sizeof (emacs_server_address.sun_path))
>
> Or, less conservatively, we could replace 10 with the actual length of
> the uid as a base-10 string.
>
> Perhaps the best approach is to use something like asprintf.  (I say
> like, because we can't use asprintf: it is not portable.  But, recent
> version of libgpg-error provide a replacement.)
>

I would humbly suggest an idiom based on snprintf(3):

    if (snprintf (emacs_server_address.sun_path,
                  sizeof(emacs_server_address.sun_path),
                  "%s/emacs%lu/%s", tmpdir, (unsigned long) uid,
                  socket_name) >= sizeof(emacs_server_address.sun_path)) {
       fprintf (stderr, "socket name is too long\n");
       free (tmpdir_storage);
       return 0;
   }

This way you don't have to worry about checking the lengths of all the
individual components of the path; you state how much space is
available and snprintf(3) tells you if it was too much.

snprintf(3) is now universally available and leads to shorter code
whose intention is clearer.  Before making this assertion I poked
around a bit for some confirmation and found a couple encouraging
things... on a NetBSD list related to Lua:
  <http://readlist.com/lists/netbsd.org/current-users/9/47416.html>
... which points at a blog post on MSDN re Microsoft support for
(among other things) snprintf(3):
  <http://blogs.msdn.com/b/vcblog/archive/2014/06/03/visual-studio-14-ctp.aspx>

So it seems previous objections to snprintf(3) based on portability
(*cough* Microsoft *cough*) are perhaps no longer valid.  It certainly
appears to be supported everywhere else (C99 was a long time ago :-).
It also appears that other groups of hackers are considering adopting
use of snprintf(3) more universally for this reason, which is a good
thing.

The same idea applies for the rest of the sprintf calls here: replace
the "check the length manually then call sprintf" idiom with "call
snprintf and let it check the length".  It is easier to audit code
like this, if nothing else.

>> [snip]

Thanks for the work on GnuPG and pinentry... M-x lurk-mode RET

Pax, -A
- --
http://trac.haqistan.net | attila at stalphonsos.com | 0xE6CC1EDB
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Processed by Mailcrypt 3.5.9 <http://mailcrypt.sourceforge.net/>

iQIcBAEBCQAGBQJVdupGAAoJEJZ30KbmzB7bEhUQAK6dG3sLBiLGOdUxQoL3s6V6
UbFEpneG6EqOSNzbzmpt37sS498CKltT+B6U3SpaJP2ty+mpQSW2xsRrmopLJK1w
9IGHHeHIjBGF55TtiI008ervV71kBHVpGnFZeDdoQF45/iUDwzXilipEJWI9n67x
XasproNc1d/DnnOEQpG0zUAqcDN5EbIofrif6B9TyBuB+8GXdMezeiVUW4dURCAO
IWV7KkSvB4tbQeL8rNelNdpUz9rjwvm8yYfWgztv2eMY7XrQ1l7oBAj4ObodB75t
Vx2qv3U9Va0bW+LZPI1Ew2d7XLAKOJ8hU1Xr/7rLZ3qHlhfTJMFuoaULgMoI+PIb
MTR3SSuM1TuTyseCNtoDG7909Gr8CF7xTx/MOz57jHqiiHhAHdMojp1JF+2hGVcP
fVY3EHB+Qpp/e3SlTqa8/2QhrTabqngZ1qMYqyV7Rmo11J5sjb4yIh078e3DSH7z
KueTSAezpBX0/LL0qPUzJIxEcXY3aSzTrOVAOTPWNAQ645FBP28njAP4qWA8m6OG
3CIK7c9C/KnvMr8GVHXbNQ5C+1U0j7quIWIuzPa8u7SdMGSM/agABg92AFDyUQJR
1mJ5mLlfJ3tVri1fTkE0u/Fatmo3IHNdEetqBrf0CQoZIU0P32E0tDsqnJ2YvPbJ
ptzgyZldSupciatlfT/+
=E1qA
-----END PGP SIGNATURE-----