Re: [PATCH 12/32] qga: replace deprecated g_get_current_time()

[Marc-André Lureau]

Hi

On Thu, Apr 7, 2022 at 9:54 AM Markus Armbruster <armbru@redhat.com> wrote:

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> According to GLib API:
> g_get_current_time has been deprecated since version 2.62 and should not
> be used in newly-written code. GTimeVal is not year-2038-safe. Use
> g_get_real_time() instead.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qga/main.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/qga/main.c b/qga/main.c
> index b9dd19918e47..1deb0ee2fbfe 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -314,7 +314,6 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
>                     const gchar *msg, gpointer opaque)
>  {
>      GAState *s = opaque;
> -    GTimeVal time;
>      const char *level_str = ga_log_level_str(level);
> 
>      if (!ga_logging_enabled(s)) {
> @@ -329,9 +328,11 @@ static void ga_log(const gchar *domain, GLogLevelFlags level,
>  #else
>      if (level & s->log_level) {
>  #endif
> -        g_get_current_time(&time);
> +        gint64 t = g_get_real_time();
>          fprintf(s->log_file,
> -                "%lu.%lu: %s: %s\n", time.tv_sec, time.tv_usec, level_str, msg);

The old code is kind of wrong.  Say it's 1649309843.000001 seconds past
the epoch.  Prints "1649309843.1".  9us later, it prints
"1649309843.10".  Should really use %06lu for the microseconds part.

good idea

Whether you want to fix this in this patch, or just note it for later in
the commit message, or ignore it alltogether is up to you.

> +                "%" G_GINT64_FORMAT ".%" G_GINT64_FORMAT

This gives me flashbacks to the 90s.  Please use PRId64 like we do
everywhere else.

I'd ditch gint64_t for int64_t, too.

ack, ack

 

> +                ": %s: %s\n", t / G_USEC_PER_SEC, t % G_USEC_PER_SEC,
> +                level_str, msg);
>          fflush(s->log_file);
>      }
>  }

--

Marc-André Lureau