[PATCH] FIX: Cast the time_t values to avoid warnings

[PATCH] FIX: Cast the time_t values to avoid warnings

[7heo]

I added 4 casts from time_t to unsigned long int
in the libxl_sprintf functions, so there is no
warning at compilation time (and no failing with
-Werror).

The casting format has been discuted, and since
there is no system having a 8 byte time_t format
yet; unsigned long int should be sufficient.
Also, it matches the libxl_sprintf syntax (%lu).

Signed-off-by: 7heo <7heo@7heo.tk>
---
 tools/libxl/libxl_create.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index cb9c822..48a60ae 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -350,7 +350,7 @@ int libxl__domain_build(libxl__gc *gc,
         vments[2] = "image/ostype";
         vments[3] = "hvm";
         vments[4] = "start_time";
-        vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        vments[5] = libxl__sprintf(gc, "%lu.%02d", (unsigned long int)(start_time.tv_sec),(int)start_time.tv_usec/10000);
 
         localents = libxl__calloc(gc, 7, sizeof(char *));
         localents[0] = "platform/acpi";
@@ -373,7 +373,7 @@ int libxl__domain_build(libxl__gc *gc,
         vments[i++] = "image/kernel";
         vments[i++] = (char *) state->pv_kernel.path;
         vments[i++] = "start_time";
-        vments[i++] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        vments[i++] = libxl__sprintf(gc, "%lu.%02d", (unsigned long int)(start_time.tv_sec),(int)start_time.tv_usec/10000);
         if (state->pv_ramdisk.path) {
             vments[i++] = "image/ramdisk";
             vments[i++] = (char *) state->pv_ramdisk.path;
@@ -846,7 +846,7 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
         vments[2] = "image/ostype";
         vments[3] = "hvm";
         vments[4] = "start_time";
-        vments[5] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        vments[5] = libxl__sprintf(gc, "%lu.%02d", (unsigned long int)(start_time.tv_sec),(int)start_time.tv_usec/10000);
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         vments = libxl__calloc(gc, 11, sizeof(char *));
@@ -856,7 +856,7 @@ void libxl__xc_domain_restore_done(libxl__egc *egc, void *dcs_void,
         vments[i++] = "image/kernel";
         vments[i++] = (char *) state->pv_kernel.path;
         vments[i++] = "start_time";
-        vments[i++] = libxl__sprintf(gc, "%lu.%02d", start_time.tv_sec,(int)start_time.tv_usec/10000);
+        vments[i++] = libxl__sprintf(gc, "%lu.%02d", (unsigned long int)(start_time.tv_sec),(int)start_time.tv_usec/10000);
         if (state->pv_ramdisk.path) {
             vments[i++] = "image/ramdisk";
             vments[i++] = (char *) state->pv_ramdisk.path;
-- 
1.7.10.4

Re: [PATCH] FIX: Cast the time_t values to avoid warnings

[7heo]

ERRATA: please read discussed insted of discuted.

Sorry for the typo.

Re: [PATCH] FIX: Cast the time_t values to avoid warnings

[Patrick Welche]

On Sat, Apr 27, 2013 at 09:52:33PM +0200, 7heo wrote:
> I added 4 casts from time_t to unsigned long int
> in the libxl_sprintf functions, so there is no
> warning at compilation time (and no failing with
> -Werror).
> 
> The casting format has been discuted, and since
> there is no system having a 8 byte time_t format
> yet; unsigned long int should be sufficient.
> Also, it matches the libxl_sprintf syntax (%lu).

I thought that earlier in the thread someone pointed
out that unsigned long long would be a better plan?
(long could just be 32 bits long)

Cheers,

Patrick

Re: [PATCH] FIX: Cast the time_t values to avoid warnings

[7heo]

On Sat, Apr 27, 2013 at 09:18:03PM +0100, Patrick Welche wrote:
> On Sat, Apr 27, 2013 at 09:52:33PM +0200, 7heo wrote:
> > I added 4 casts from time_t to unsigned long int
> > in the libxl_sprintf functions, so there is no
> > warning at compilation time (and no failing with
> > -Werror).
> > 
> > The casting format has been discuted, and since
> > there is no system having a 8 byte time_t format
> > yet; unsigned long int should be sufficient.
> > Also, it matches the libxl_sprintf syntax (%lu).
> 
> I thought that earlier in the thread someone pointed
> out that unsigned long long would be a better plan?
> (long could just be 32 bits long)
> 
> Cheers,
> 
> Patrick

As explained in the second paragraph of the git commit
message (even if I did a typo); this has been discussed
already.

Regards,
7heo.

Re: [PATCH] FIX: Cast the time_t values to avoid warnings

[Patrick Welche]

On Sat, Apr 27, 2013 at 11:32:35PM +0200, 7heo@7heo.tk wrote:
> On Sat, Apr 27, 2013 at 09:18:03PM +0100, Patrick Welche wrote:
> > On Sat, Apr 27, 2013 at 09:52:33PM +0200, 7heo wrote:
> > > I added 4 casts from time_t to unsigned long int
> > > in the libxl_sprintf functions, so there is no
> > > warning at compilation time (and no failing with
> > > -Werror).
> > > 
> > > The casting format has been discuted, and since
> > > there is no system having a 8 byte time_t format
> > > yet; unsigned long int should be sufficient.
> > > Also, it matches the libxl_sprintf syntax (%lu).
> > 
> > I thought that earlier in the thread someone pointed
> > out that unsigned long long would be a better plan?
> > (long could just be 32 bits long)
> > 
> > Cheers,
> > 
> > Patrick
> 
> As explained in the second paragraph of the git commit
> message (even if I did a typo); this has been discussed
> already.

Is the typo in the part which says "since there is no system having
a 8 byte time_t format yet", which should read "most systems which
want to keep going beyond 2038 have 8 byte time_t format"?
The box I'm sitting in front of certainly has sizeof(time_t)==8.

The point is that all that is guaranteed is that
  sizeof(long long) >= sizeof(long) >= sizeof(int)

Just checked on a 32-bit OS:

% cat foo.c
#include <stdio.h>
#include <time.h>

int main()
{
  printf("time_t: %u  int: %u  long: %u  long long: %u\n",
    sizeof(time_t), sizeof(int), sizeof(long), sizeof(long long));

  return 0;
}
% ./foo
time_t: 8  int: 4  long: 4  long long: 8

so long long is a better choice.


Cheers,

Patrick

Re: [PATCH] FIX: Cast the time_t values to avoid warnings

[7heo]

On Sat, Apr 27, 2013 at 11:03:49PM +0100, Patrick Welche wrote:
> On Sat, Apr 27, 2013 at 11:32:35PM +0200, 7heo@7heo.tk wrote:
> > On Sat, Apr 27, 2013 at 09:18:03PM +0100, Patrick Welche wrote:
> > > On Sat, Apr 27, 2013 at 09:52:33PM +0200, 7heo wrote:
> > > > I added 4 casts from time_t to unsigned long int
> > > > in the libxl_sprintf functions, so there is no
> > > > warning at compilation time (and no failing with
> > > > -Werror).
> > > > 
> > > > The casting format has been discuted, and since
> > > > there is no system having a 8 byte time_t format
> > > > yet; unsigned long int should be sufficient.
> > > > Also, it matches the libxl_sprintf syntax (%lu).
> > > 
> > > I thought that earlier in the thread someone pointed
> > > out that unsigned long long would be a better plan?
> > > (long could just be 32 bits long)
> > > 
> > > Cheers,
> > > 
> > > Patrick
> > 
> > As explained in the second paragraph of the git commit
> > message (even if I did a typo); this has been discussed
> > already.
> 
> Is the typo in the part which says "since there is no system having
> a 8 byte time_t format yet", which should read "most systems which
> want to keep going beyond 2038 have 8 byte time_t format"?
> The box I'm sitting in front of certainly has sizeof(time_t)==8.
> 
> The point is that all that is guaranteed is that
>   sizeof(long long) >= sizeof(long) >= sizeof(int)
> 
> Just checked on a 32-bit OS:
> 
> % cat foo.c
> #include <stdio.h>
> #include <time.h>
> 
> int main()
> {
>   printf("time_t: %u  int: %u  long: %u  long long: %u\n",
>     sizeof(time_t), sizeof(int), sizeof(long), sizeof(long long));
> 
>   return 0;
> }
> % ./foo
> time_t: 8  int: 4  long: 4  long long: 8
> 
> so long long is a better choice.
> 
> 
> Cheers,
> 
> Patrick

Thanks for having taken the time to check. Then it would
maybe make sense to check the size of time_t at
compilation time (with cpp instructions) in order to
chose the cast that fits, wouldn't it?

Also, this surprises me, I wouldn't have imagined that
people would care about the 32bit time overflow before
2035...

Regards,
7heo.

Re: [PATCH] FIX: Cast the time_t values to avoid warnings

[Alex Bligh]

--On 28 April 2013 00:27:29 +0200 7heo@7heo.tk wrote:

> Thanks for having taken the time to check. Then it would
> maybe make sense to check the size of time_t at
> compilation time (with cpp instructions) in order to
> chose the cast that fits, wouldn't it?

Would it not be easier just to cast to unsigned long long, and use %llu?

-- 
Alex Bligh

Re: [PATCH] FIX: Cast the time_t values to avoid warnings

[7heo]

On Sat, Apr 27, 2013 at 11:59:35PM +0100, Alex Bligh wrote:
> 
> 
> --On 28 April 2013 00:27:29 +0200 7heo@7heo.tk wrote:
> 
> >Thanks for having taken the time to check. Then it would
> >maybe make sense to check the size of time_t at
> >compilation time (with cpp instructions) in order to
> >chose the cast that fits, wouldn't it?
> 
> Would it not be easier just to cast to unsigned long long, and use %llu?
> 
> -- 
> Alex Bligh

Yeah, you're right. I'm then going to do that.

Thanks for the suggestion.

Regards,
7heo.