On Tue, 25.03.14 02:13, David Härdeman (da...@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 _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
