[Cluster-devel] [PATCH 1/4] liblogthread: call to localtime needs return value check

[Steven Whitehouse <swhiteho@redhat.com>]

Hi,

On Mon, 2011-10-10 at 10:45 +0200, Fabio M. Di Nitto wrote:
> Spotted by Coverity Scan
> 
> Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
> ---
> :100644 100644 ba96a2a... 4f8354a... M	common/liblogthread/liblogthread.c
>  common/liblogthread/liblogthread.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/common/liblogthread/liblogthread.c b/common/liblogthread/liblogthread.c
> index ba96a2a..4f8354a 100644
> --- a/common/liblogthread/liblogthread.c
> +++ b/common/liblogthread/liblogthread.c
> @@ -42,8 +42,15 @@ static FILE *logt_logfile_fp;
>  static char *_time(time_t *t)
>  {
>  	static char buf[64];
> +	struct tm *tm;
> +
> +	tm = localtime(t);
> +	if (!tm) {
> +		strncpy(buf, "unknown time", sizeof(buf) - 1);
> +	} else {
> +		strftime(buf, sizeof(buf), "%b %d %T", tm);
> +	}
>  
> -	strftime(buf, sizeof(buf), "%b %d %T", localtime(t));
>  	return buf;
>  }
>  

I'm wondering under just what circumstances localtime would return
NULL... since the returned buffer is static, there is no allocation that
could fail. Since the time_t is just seconds since the epoc, all
possible values are valid. Maybe if it was passed a NULL time_t *
perhaps, but that is, I suspect impossible.

So I'm wondering whether this is a false positive,

Steve.