Re: [libvirt] [PATCH v3 09/34] Adapt to VIR_STRDUP and VIR_STRNDUP in src/lxc/*

On Thu, May 09, 2013 at 12:31:14PM +0200, Michal Privoznik wrote: > On 09.05.2013 10:15, Daniel P. Berrange wrote: >> On Wed, May 08, 2013 at 04:01:04PM -0600, Eric Blake wrote: >>> On 05/03/2013 08:53 AM, Michal Privoznik wrote: >>>> --- >>>> src/lxc/lxc_cgroup.c | 3 +-- >>>> src/lxc/lxc_conf.c | 36 +++++++++++++++++------------------- >>>> src/lxc/lxc_container.c | 18 ++++-------------- >>>> src/lxc/lxc_controller.c | 20 ++++++++------------ >>>> src/lxc/lxc_driver.c | 9 ++------- >>>> src/lxc/lxc_process.c | 7 ++++--- >>>> 6 files changed, 36 insertions(+), 57 deletions(-) >>>> >>>> diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c >>>> @@ -161,18 +161,18 @@ int lxcLoadDriverConfig(virLXCDriverPtr driver) >>>> driver->securityRequireConfined = false; >>>> >>>> /* Set the container configuration directory */ >>>> - if ((driver->configDir = strdup(LXC_CONFIG_DIR)) == NULL) >>>> - goto no_memory; >>>> - if ((driver->stateDir = strdup(LXC_STATE_DIR)) == NULL) >>>> - goto no_memory; >>>> - if ((driver->logDir = strdup(LXC_LOG_DIR)) == NULL) >>>> - goto no_memory; >>>> - if ((driver->autostartDir = strdup(LXC_AUTOSTART_DIR)) == NULL) >>>> - goto no_memory; >>>> + if (VIR_STRDUP(driver->configDir, LXC_CONFIG_DIR) < 0) >>>> + goto error; >>>> + if (VIR_STRDUP(driver->stateDir, LXC_STATE_DIR) < 0) >>>> + goto error; >>>> + if (VIR_STRDUP(driver->logDir, LXC_LOG_DIR) < 0) >>>> + goto error; >>>> + if (VIR_STRDUP(driver->autostartDir, LXC_AUTOSTART_DIR) < 0) >>>> + goto error; >>> >>> You could chain these into one 'if', if desired. >> >> I generally prefer it if we do not merge separate statements >> like these into one if, because it makes debugging harder. >> eg if any of these were to generate a SEGV, then GDB can >> only tell you the line of the start of the if(). Having >> them separate it can tell you exactly which is at fault. >> >> Daniel >> > > Not true unless you compile without -g. If you do copile with -g gdb is > actually able to tell you the full stack trace - so you can see the > right line within if() statement which caused the trouble. Actually it depends on the quality of the debuginfo provided by the compiler. Historically it has not been able todo this correctly. I am not clear on what version of gcc fixed this, but I've certainly hit the problem many times in the past. Daniel