All of lore.kernel.org
 help / color / mirror / Atom feed
* [mdadm PATCH 1/1] Fix some type comparison problems
@ 2016-02-06  1:18 Xiao Ni
  2016-02-08 15:51 ` Jes Sorensen
  0 siblings, 1 reply; 7+ messages in thread
From: Xiao Ni @ 2016-02-06  1:18 UTC (permalink / raw)
  To: linux-raid; +Cc: Jes.Sorensen, deepa.kernel, maxin.john

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [mdadm PATCH 1/1] Fix some type comparison problems
  2016-02-06  1:18 [mdadm PATCH 1/1] Fix some type comparison problems Xiao Ni
@ 2016-02-08 15:51 ` Jes Sorensen
  0 siblings, 0 replies; 7+ messages in thread
From: Jes Sorensen @ 2016-02-08 15:51 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid, deepa.kernel, maxin.john

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;

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [mdadm PATCH 1/1] Fix some type comparison problems
  2016-02-04 12:26 Xiao Ni
@ 2016-02-04 13:12 ` Jes Sorensen
  0 siblings, 0 replies; 7+ messages in thread
From: Jes Sorensen @ 2016-02-04 13:12 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid, deepa.kernel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [mdadm PATCH 1/1] Fix some type comparison problems
@ 2016-02-04 12:26 Xiao Ni
  2016-02-04 13:12 ` Jes Sorensen
  0 siblings, 1 reply; 7+ messages in thread
From: Xiao Ni @ 2016-02-04 12:26 UTC (permalink / raw)
  To: linux-raid; +Cc: Jes.Sorensen, deepa.kernel

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [mdadm PATCH 1/1] Fix some type comparison problems
  2016-02-03 20:36 ` Jes Sorensen
@ 2016-02-04  3:10   ` Deepa Dinamani
  0 siblings, 0 replies; 7+ messages in thread
From: Deepa Dinamani @ 2016-02-04  3:10 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: Xiao Ni, linux-raid, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [mdadm PATCH 1/1] Fix some type comparison problems
  2016-02-03  5:24 Xiao Ni
@ 2016-02-03 20:36 ` Jes Sorensen
  2016-02-04  3:10   ` Deepa Dinamani
  0 siblings, 1 reply; 7+ messages in thread
From: Jes Sorensen @ 2016-02-03 20:36 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid, Deepa Dinamani

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [mdadm PATCH 1/1] Fix some type comparison problems
@ 2016-02-03  5:24 Xiao Ni
  2016-02-03 20:36 ` Jes Sorensen
  0 siblings, 1 reply; 7+ messages in thread
From: Xiao Ni @ 2016-02-03  5:24 UTC (permalink / raw)
  To: linux-raid; +Cc: Jes.Sorensen

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-02-08 15:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-06  1:18 [mdadm PATCH 1/1] Fix some type comparison problems Xiao Ni
2016-02-08 15:51 ` Jes Sorensen
  -- strict thread matches above, loose matches on Subject: below --
2016-02-04 12:26 Xiao Ni
2016-02-04 13:12 ` Jes Sorensen
2016-02-03  5:24 Xiao Ni
2016-02-03 20:36 ` Jes Sorensen
2016-02-04  3:10   ` Deepa Dinamani

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.