Re: [PATCH] uptime: fix a memory leak after printing results

[Pádraig Brady]

On 07/06/2013 02:13 PM, Anton Ovchinnikov wrote:
> The issue was found after checking with 'valgrind'.
> BTW, several other programs also have some memory issues. I can take a
> look at them as well in the nearest future.

Cool thanks.
Note this is flagged as "definitely lost" by valgrind,
which is something we'd want to suppress.
However in this case it's a redundant free() as the
function is only called once before exit().
For such cases we usually do:

IF_LINT (free (utmp_buf));

or for larger blocks:

#ifdef lint
  ....
#endif

Note that "lint" is defined automatically when
you ./configure --enable-gcc-warnings
Note also that that is enabled automatically when
configure is run from a git repo with gcc >= 4.6.0

> --
> Best regards,
> Anton
> 
> 
>>From 19a75ddf4394443c4253323a20508943f0a607c8 Mon Sep 17 00:00:00 2001
> From: Anton Ovchinnikov <address@hidden>
> Date: Sat, 6 Jul 2013 15:00:02 +0400
> Subject: [PATCH] uptime: fix a memory leak after printing results

Since this isn't a functional change to uptime,
we prefer to make that evident in the summary line by instead using:

maint: avoid a memory leak warning from valgrind

> * src/uptime.c (uptime): Free utmp_buf returned from read_utmp.
> Initialize utmp_buf and n_users explicitly to avoid an uninitialized

This description had me worried that we were previously
using n_users uninitialized.  As that's not the case
I'll remove that redundant initialization and mention in the commit.

> usage when (HAVE_UTMPX_H || HAVE_UTMP_H) macro condition is not met.
> ---
>  src/uptime.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/uptime.c b/src/uptime.c
> index 8e8f2ca..642d9d1 100644
> --- a/src/uptime.c
> +++ b/src/uptime.c
> @@ -175,8 +175,8 @@ print_uptime (size_t n, const STRUCT_UTMP *this)
>  static void
>  uptime (const char *filename, int options)
>  {
> -  size_t n_users;
> -  STRUCT_UTMP *utmp_buf;
> +  size_t n_users = 0;
> +  STRUCT_UTMP *utmp_buf = NULL;
> 
>  #if HAVE_UTMPX_H || HAVE_UTMP_H
>    if (read_utmp (filename, &n_users, &utmp_buf, options) != 0)
> @@ -184,6 +184,7 @@ uptime (const char *filename, int options)
>  #endif
> 
>    print_uptime (n_users, utmp_buf);
> +  free (utmp_buf);
>  }

Pushed with the above mentioned adjustments.

thanks!
Pádraig.