[systemd-devel] [PATCH 1/3] Add more password agent information

[Lennart Poettering]

On Tue, 25.03.14 02:13, David Härdeman (david at hardeman.nu) wrote:

> >Also, please use strappend() for cases like this, where we just want to
> >concatenate two strings.
> 
> Hmmm, the asprinf use there matches the style of the code of the rest of
> the function....for example, with the patch applied that part reads:

Yeah, the password query stuff is relatively old code, and strappend()
is a more recent addition...

We try to avoid asprintf() where possible, on the simple grounds that is
is measurebly slow. In this case that doesn't matter much though, since
it's nothing we execute repeatedly.

Anyway, it's nitpicking, doesn't really matter and I'd merge it
anyway...

Oh, one thing though: if the concatenated strings are known to be
limited in size, then strappenda() is a better option actually than
either asprintf() or strappend(). It allocates on the stack, and thus
doesn't fail, and requires no checking nor freeing. Doesn't work inside
of loops though.

> if (asprintf(&text, "Please enter passphrase for disk %s!", name) < 0)
> 	return log_oom();
> 
> if (asprintf(&id, "cryptsetup:%s", name) < 0)
> 	return log_oom();
> 
> Changing the second asprintf to use strappend and cescape wouldn't
> really make it more readable, would it?

If the id concat/escape stuff is needed multiple times, it might be a
good idea to turn it into a function of its own...

Lennart

-- 
Lennart Poettering, Red Hat