[mdadm PATCH 1/1] Fix some type comparison problems

[mdadm PATCH 1/1] Fix some type comparison problems

[Xiao Ni]

It complains when building on s390 and i686 platform.

Signed-off-by : Xiao Ni <xni@redhat.com>
---
 Monitor.c | 2 +-
 util.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index f19c2e5..928aa45 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -542,7 +542,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		alert("NewArray", st->devname, NULL, ainfo);
 	}
 
-	if (st->utime == array.utime &&
+	if (st->utime == (long)array.utime &&
 	    st->failed == array.failed_disks &&
 	    st->working == array.working_disks &&
 	    st->spare == array.spare_disks &&
diff --git a/util.c b/util.c
index 970d484..ccbb2bc 100644
--- a/util.c
+++ b/util.c
@@ -1288,7 +1288,7 @@ struct supertype *guess_super_type(int fd, enum guess_types guess_type)
 			struct mdinfo info;
 			st->ss->getinfo_super(st, &info, NULL);
 			if (bestsuper == -1 ||
-			    besttime < info.array.ctime) {
+			    besttime < (time_t)info.array.ctime) {
 				bestsuper = i;
 				besttime = info.array.ctime;
 			}
-- 
2.4.3

Re: [mdadm PATCH 1/1] Fix some type comparison problems

[Jes Sorensen]

Xiao Ni <xni@redhat.com> writes:
> It complains when building on s390 and i686 platform.
>
> Signed-off-by : Xiao Ni <xni@redhat.com>
> ---
>  Monitor.c | 2 +-
>  util.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Monitor.c b/Monitor.c
> index f19c2e5..928aa45 100644
> --- a/Monitor.c
> +++ b/Monitor.c
> @@ -542,7 +542,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
>  		alert("NewArray", st->devname, NULL, ainfo);
>  	}
>  
> -	if (st->utime == array.utime &&
> +	if (st->utime == (long)array.utime &&
>  	    st->failed == array.failed_disks &&
>  	    st->working == array.working_disks &&
>  	    st->spare == array.spare_disks &&

Hi Xiao,

Thanks for the patch - I think the correct fix in Monitor.c is to change
utime in struct state from long to unsigned int, to match the change
Deepa Dinamani introduced in 26714713cd2bad9e0bf7f4669f6cc4659ceaab6c
utime is only ever derived from mdu_array_info_s, so it should be safe.

> diff --git a/util.c b/util.c
> index 970d484..ccbb2bc 100644
> --- a/util.c
> +++ b/util.c
> @@ -1288,7 +1288,7 @@ struct supertype *guess_super_type(int fd, enum guess_types guess_type)
>  			struct mdinfo info;
>  			st->ss->getinfo_super(st, &info, NULL);
>  			if (bestsuper == -1 ||
> -			    besttime < info.array.ctime) {
> +			    besttime < (time_t)info.array.ctime) {
>  				bestsuper = i;
>  				besttime = info.array.ctime;
>  			}

Here I think we should change the declaration of besttime from time_t to
unsigned int, again to match the definition used in mdu_array_info_s.

That or we should change mdu_array_info_s to use time_t instead, but I
am not sure that would be safe.

Cheers,
Jes

Re: [mdadm PATCH 1/1] Fix some type comparison problems

[Deepa Dinamani]

On Wed, Feb 3, 2016 at 12:36 PM, Jes Sorensen <Jes.Sorensen@redhat.com> wrote:
> Xiao Ni <xni@redhat.com> writes:
>> It complains when building on s390 and i686 platform.
>>
>> Signed-off-by : Xiao Ni <xni@redhat.com>
>> ---
>>  Monitor.c | 2 +-
>>  util.c    | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Monitor.c b/Monitor.c
>> index f19c2e5..928aa45 100644
>> --- a/Monitor.c
>> +++ b/Monitor.c
>> @@ -542,7 +542,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
>>               alert("NewArray", st->devname, NULL, ainfo);
>>       }
>>
>> -     if (st->utime == array.utime &&
>> +     if (st->utime == (long)array.utime &&
>>           st->failed == array.failed_disks &&
>>           st->working == array.working_disks &&
>>           st->spare == array.spare_disks &&
>
> Hi Xiao,
>
> Thanks for the patch - I think the correct fix in Monitor.c is to change
> utime in struct state from long to unsigned int, to match the change
> Deepa Dinamani introduced in 26714713cd2bad9e0bf7f4669f6cc4659ceaab6c
> utime is only ever derived from mdu_array_info_s, so it should be safe.
>
>> diff --git a/util.c b/util.c
>> index 970d484..ccbb2bc 100644
>> --- a/util.c
>> +++ b/util.c
>> @@ -1288,7 +1288,7 @@ struct supertype *guess_super_type(int fd, enum guess_types guess_type)
>>                       struct mdinfo info;
>>                       st->ss->getinfo_super(st, &info, NULL);
>>                       if (bestsuper == -1 ||
>> -                         besttime < info.array.ctime) {
>> +                         besttime < (time_t)info.array.ctime) {
>>                               bestsuper = i;
>>                               besttime = info.array.ctime;
>>                       }
>
> Here I think we should change the declaration of besttime from time_t to
> unsigned int, again to match the definition used in mdu_array_info_s.
>
> That or we should change mdu_array_info_s to use time_t instead, but I
> am not sure that would be safe.

It should be changed to unsigned int.
time_t is not long enough to hold timestamps after the year
2038(INT_MAX) as long is 32 bits on 32 bit systems.
It was changed to unsigned int to extend it until the year 2106.

-Deepa

[mdadm PATCH 1/1] Fix some type comparison problems

[Xiao Ni]

It complains when building on s390 and i686 platform.

Signed-off-by : Xiao Ni <xni@redhat.com>
---
 Monitor.c | 2 +-
 util.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index f19c2e5..9a6cf7e 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -542,7 +542,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 		alert("NewArray", st->devname, NULL, ainfo);
 	}
 
-	if (st->utime == array.utime &&
+	if ((unsigned int)st->utime == array.utime &&
 	    st->failed == array.failed_disks &&
 	    st->working == array.working_disks &&
 	    st->spare == array.spare_disks &&
diff --git a/util.c b/util.c
index 970d484..6e7d3fb 100644
--- a/util.c
+++ b/util.c
@@ -1267,7 +1267,7 @@ struct supertype *guess_super_type(int fd, enum guess_types guess_type)
 	 */
 	struct superswitch  *ss;
 	struct supertype *st;
-	time_t besttime = 0;
+	unsigned int besttime = 0;
 	int bestsuper = -1;
 	int i;
 
-- 
2.4.3

Re: [mdadm PATCH 1/1] Fix some type comparison problems

[Jes Sorensen]

Xiao Ni <xni@redhat.com> writes:
> It complains when building on s390 and i686 platform.
>
> Signed-off-by : Xiao Ni <xni@redhat.com>

Hi Xiao,

The description needs to be a little clearer - something more like
"Change utime to unsigned int to be able to hold times until 2106."

> ---
>  Monitor.c | 2 +-
>  util.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Monitor.c b/Monitor.c
> index f19c2e5..9a6cf7e 100644
> --- a/Monitor.c
> +++ b/Monitor.c
> @@ -542,7 +542,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
>  		alert("NewArray", st->devname, NULL, ainfo);
>  	}
>  
> -	if (st->utime == array.utime &&
> +	if ((unsigned int)st->utime == array.utime &&
>  	    st->failed == array.failed_disks &&
>  	    st->working == array.working_disks &&
>  	    st->spare == array.spare_disks &&

Instead of the cast, please change the type in struct state.

> diff --git a/util.c b/util.c
> index 970d484..6e7d3fb 100644
> --- a/util.c
> +++ b/util.c
> @@ -1267,7 +1267,7 @@ struct supertype *guess_super_type(int fd, enum guess_types guess_type)
>  	 */
>  	struct superswitch  *ss;
>  	struct supertype *st;
> -	time_t besttime = 0;
> +	unsigned int besttime = 0;
>  	int bestsuper = -1;
>  	int i;

This one is fine.

Thanks,
Jes

[mdadm PATCH 1/1] Fix some type comparison problems

[Xiao Ni]

As 26714713cd2bad9e0bf7f4669f6cc4659ceaab6c said, 32 bit signed 
timestamps will overflow in the year 2038. It already changed the
utime and ctime in struct mdu_array_info_s from int to unsigned 
int. So we need to change the values that compared with them to
unsigned int too.

Signed-off-by : Xiao Ni <xni@redhat.com>
---
 Monitor.c | 2 +-
 util.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index f19c2e5..6df80f9 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -33,7 +33,7 @@
 struct state {
 	char *devname;
 	char devnm[32];	/* to sync with mdstat info */
-	long utime;
+	unsigned int utime;
 	int err;
 	char *spare_group;
 	int active, working, failed, spare, raid;
diff --git a/util.c b/util.c
index 970d484..6e7d3fb 100644
--- a/util.c
+++ b/util.c
@@ -1267,7 +1267,7 @@ struct supertype *guess_super_type(int fd, enum guess_types guess_type)
 	 */
 	struct superswitch  *ss;
 	struct supertype *st;
-	time_t besttime = 0;
+	unsigned int besttime = 0;
 	int bestsuper = -1;
 	int i;
 
-- 
2.4.3

Re: [mdadm PATCH 1/1] Fix some type comparison problems

[Jes Sorensen]

Xiao Ni <xni@redhat.com> writes:
> As 26714713cd2bad9e0bf7f4669f6cc4659ceaab6c said, 32 bit signed 
> timestamps will overflow in the year 2038. It already changed the
> utime and ctime in struct mdu_array_info_s from int to unsigned 
> int. So we need to change the values that compared with them to
> unsigned int too.
>
> Signed-off-by : Xiao Ni <xni@redhat.com>
> ---
>  Monitor.c | 2 +-
>  util.c    | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Applied!

Thanks,
Jes

> diff --git a/Monitor.c b/Monitor.c
> index f19c2e5..6df80f9 100644
> --- a/Monitor.c
> +++ b/Monitor.c
> @@ -33,7 +33,7 @@
>  struct state {
>  	char *devname;
>  	char devnm[32];	/* to sync with mdstat info */
> -	long utime;
> +	unsigned int utime;
>  	int err;
>  	char *spare_group;
>  	int active, working, failed, spare, raid;
> diff --git a/util.c b/util.c
> index 970d484..6e7d3fb 100644
> --- a/util.c
> +++ b/util.c
> @@ -1267,7 +1267,7 @@ struct supertype *guess_super_type(int fd, enum guess_types guess_type)
>  	 */
>  	struct superswitch  *ss;
>  	struct supertype *st;
> -	time_t besttime = 0;
> +	unsigned int besttime = 0;
>  	int bestsuper = -1;
>  	int i;