On Fri, Apr 1, 2011 at 5:30 AM, Andrew Beekhof <andrew(a)beekhof.net> wrote:
On Sun, Mar 27, 2011 at 3:08 AM, Russell Bryant
<russell(a)russellbryant.net> wrote:
> 1) Fix memory leaks in host_get_operating_system() and
> host_get_architecture(). The code was written as if the local data
> buffer was static, but it was not.
ack
>
> 2) Minor optimizations - localize variables, only call init() when
> the local buffer has not already been populated.
tbh, I'm not sure this is worth the extra lines and it means the
callers need to know how init() is implemented.
Fair enough. It's an extra function call but moving it is a micro
optimization anyway.
> 3) One formatting tweak to move a return to its own line.
agreed this is preferred
>
> Signed-off-by: Russell Bryant <russell(a)russellbryant.net>
> ---
> src/lib/host.c | 20 +++++++++++++-------
> 1 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/src/lib/host.c b/src/lib/host.c
> index 73e1f6a..cd40db2 100644
> --- a/src/lib/host.c
> +++ b/src/lib/host.c
> @@ -47,7 +47,9 @@ cpuinfo_t cpuinfo = { NULL, 0, 0 };
> static void
> init(void)
> {
> - if(host_init.sigar_init) return;
> + if(host_init.sigar_init) {
> + return;
> + }
> sigar_open(&host_init.sigar);
> host_init.sigar_init = TRUE;
> }
> @@ -67,14 +69,16 @@ host_get_hostname(void)
> const char *
> host_get_operating_system(void)
> {
> - init();
> - sigar_sys_info_t sysinfo;
> - char *operating_system = NULL;
> + static const char *operating_system = NULL;
>
> if(operating_system == NULL) {
> + sigar_sys_info_t sysinfo;
> +
> + init();
> sigar_sys_info_get(host_init.sigar, &sysinfo);
> operating_system = g_strdup_printf("%s (%s)", sysinfo.vendor_name,
sysinfo.version);
> }
> +
> return operating_system;
> }
>
> @@ -87,14 +91,16 @@ host_get_cpu_wordsize(void)
> const char *
> host_get_architecture(void)
> {
> - init();
> - sigar_sys_info_t sysinfo;
> - char *arch = NULL;
> + static const char *arch = NULL;
>
> if(arch == NULL) {
> + sigar_sys_info_t sysinfo;
> +
> + init();
> sigar_sys_info_get(host_init.sigar, &sysinfo);
> arch = g_strdup(sysinfo.arch);
> }
> +
> return arch;
> }
>
> --
> 1.7.4
>
> _______________________________________________
> Matahari mailing list
> Matahari(a)lists.fedorahosted.org
>
https://fedorahosted.org/mailman/listinfo/matahari
>