> Am 31.07.2016 um 00:51 schrieb Ingo Schwarze <schwa...@usta.de>: > > Hi Theo, > > Theo Buehler wrote on Sat, Jul 30, 2016 at 09:33:01PM +0200: > >> ++#ifdef HAVE_EXPLICIT_BZERO >> ++#define CLEAR_PASSWORD(p, len) explicit_bzero((p), (len)) >> ++#else >> ++#define CLEAR_PASSWORD(p, len) memset((p), 0, (len)) >> ++#endif > > Not to be a prick or to start a bikeshed - i'm not opposed to your > patch. Just for consideration, more for encouraging best practices > in the general case than in this specific example. > > I think we should encourage upstream projects to handle portability > in the following way: > > 1. Fully commit to using good interfaces in the main code, > no matter whether they are (already) standardized. > NEVER use wrappers around standard interfaces, and AVOID > wrappers even around good non-standard interfaces. > 2. Instead, provide replacement implementations in compat_*.c > files at the edge of the upstream codebase, if possible > containing just a copy of the best available implementation, > if impossible containing an implementation with reduced > functionality, in extreme cases where even that is > impossible, like for pledge(2), a no-op stub. > > Hence, why not: > > [...] >> +@@ -172,10 +178,12 @@ readpw(Display *dpy, const char *pws) >> + break; >> + case XK_Escape: >> + len = 0; >> ++ explicit_bzero(passwd, sizeof(passwd)); >> + break; > > and so on? Encourage upstream to take that as-is. > > Proposing HAVE_EXPLICIT_BZERO to upstream is very good. Do that! > > Then encourage upstream to:
First, I personally fully agree with you. But the slock port is a suckless.org tool and these tools follow own rules, e.g. no real configuration system, usually a single or very few source files (with very low overall SLOC), and keep things as trivial as possible. Go have a look at their sources in repo e.g. dwm, dmenu, sic, st, tabbed, ... and you'll see the pattern. http://git.suckless.org > 1. Include a file compat_explicit_bzero.c into their tarball. You can't dictate upstream how they should organize their code. > Ideally, talk to some of our C wizards (like tedu@) how > a portable implementation should look like. I suspect > that src/lib/libc/string/explicit_bzero.c will not be > very portable and maybe not even work outside libc. > 2. Include a configuration test checking at least whether the Upstream does not really use a configuration system. > operating system provides a function explicit_bzero(3) with the > correct prototype in the correct header, ideally also whether > it actually zeros the memory in some situation where some > compilers would optimize a memset(3) away. > 3. If step 2 succeeds, It won't. > let whatever configuration system upstream > uses set HAVE_EXPLICIT_BZERO; otherwise, let it clear > HAVE_EXPLICIT_BZERO, thus choosing which implementation > to use. Look at > http://mdocml.bsd.lv/cgi-bin/cvsweb/?cvsroot=mdocml > For many examples of test-*.c files and compat_*.c > implementations, though admittely not including > explicit_bzero(3). > > If upstream does not want advice 1, tell them to use this: > >> ++#ifndef HAVE_EXPLICIT_BZERO >> ++#define explicit_bzero(p, len) memset((p), 0, (len)) >> ++#endif > Yours, > Ingo
