"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: > On Mon, Feb 09, 2009 at 03:39:15PM +0100, Jim Meyering wrote: >> "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: >> >> > On Mon, Feb 09, 2009 at 02:01:19PM +0100, Jim Meyering wrote: >> >> However, first things first: >> >> >> >> Here's a patch that adds two blocks, neither pretty, >> >> but with less duplication than the 3rd alternative, >> >> which duplicates both the snprintf and the result comparison. >> >> (of course, I'll use only one of them) >> >> >> >> BTW, this also eliminates the uses of PATH_MAX that were >> >> vestiges of a messy rebase. Now we test against maxlen. >> >> >> >> Of these two, I prefer the latter (slightly less duplication). >> >> Do you care? >> > >> > I don't particular like either option - too much line noise in there. >> > Two thoughts - printf is redundant in the first place, since the compiler Um. checking snprintf's result was not redundant. >> > will happily concatenate 2 static strings, so we can just strdup(). In >> > the second case, we could asprintf instead, and so have something like >> > >> > if (uid == SYSTEM_UID) >> > server->logDir = strdup(LOCAL_STATE_DIR "/log/libvirt") >> > else >> > virAsprintf(&server->logDir, "%s/.libvirt/log", dir_prefix)) >> > >> > if (!server->logDir) >> > ... oom handling ... >> >> Can't do that. The logDir member is declared like this: >> >> qemud/qemud.h: char logDir[PATH_MAX]; > > No, I meant change that to just 'char *logDir' since pre-declaring > it PATH_MAX isn't really helping to simplify the code at all. In that case, no problem. In that context, your saying printf is redundant makes sense ;-) Will do. Besides, that's more consistent with what's done e.g., in src/lxc_conf.h: char *logDir; src/qemu_conf.h: char *logDir; src/uml_conf.h: char *logDir; -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list