2009-07-20 02:56:36

by Zhao Lei

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add function to convert between calendar time andbroken-down time for universal use

* From: "Pavel Machek" <[email protected]>
>> > +extern void gmtime(__kernel_time_t totalsecs,
>> > + unsigned int *year, unsigned int *mon, unsigned int *mday,
>> > + unsigned int *hour, unsigned int *min, unsigned int *sec,
>> > + unsigned int *wday, unsigned int *yday);
>> > +extern void localtime(__kernel_time_t totalsecs,
>> > + unsigned int *year, unsigned int *mon, unsigned int *mday,
>> > + unsigned int *hour, unsigned int *min, unsigned int *sec,
>> > + unsigned int *wday, unsigned int *yday);
>
>
> Should year/mon/.../yday be passed up as a structure?

Hello, Pavel

Thanks for your attention.

Actually, I considered to introduce a struct as your think, but finally I
choose to use arguments list instead of a struct, because:
1: User can easy to call this function without define a struct
2: Get rid of adding a additional struct into kernel

In fact, I think both(use a struct or not) should be ok.

Thanks
Zhaolei
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?


2009-07-20 03:21:27

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/2] Add function to convert between calendar time andbroken-down time for universal use

On Mon, 20 Jul 2009 10:56:55 +0800 "Zhaolei" <[email protected]> wrote:

> * From: "Pavel Machek" <[email protected]>
> >> > +extern void gmtime(__kernel_time_t totalsecs,
> >> > + unsigned int *year, unsigned int *mon, unsigned int *mday,
> >> > + unsigned int *hour, unsigned int *min, unsigned int *sec,
> >> > + unsigned int *wday, unsigned int *yday);
> >> > +extern void localtime(__kernel_time_t totalsecs,
> >> > + unsigned int *year, unsigned int *mon, unsigned int *mday,
> >> > + unsigned int *hour, unsigned int *min, unsigned int *sec,
> >> > + unsigned int *wday, unsigned int *yday);
> >
> >
> > Should year/mon/.../yday be passed up as a structure?
>
> Hello, Pavel
>
> Thanks for your attention.
>
> Actually, I considered to introduce a struct as your think, but finally I
> choose to use arguments list instead of a struct, because:
> 1: User can easy to call this function without define a struct
> 2: Get rid of adding a additional struct into kernel
>
> In fact, I think both(use a struct or not) should be ok.

Using a struct will generate better code at caller sites and possibly
at the called site too. The compiler doesn't need to marshal those
eight pointers on the stack (or wherever the architecture puts them),
possibly less registers will be consumed, etc. And it'll use less
stack space.

So I do think it would be better from that point of view.

However it will probably require much more code change at the call
sites. But those changes will be simple, and probably a good thing
regardless.

2009-07-20 09:54:41

by Zhao Lei

[permalink] [raw]
Subject: [PATCH v3 0/2] Add function to convert between calendar time and broken-down time for universal use

Hello, Andrew

This is Patch v3.
I decide to use a struct instead of argument list to store year/mon/.../yday.

Changelog v2->v3:
Using a struct to save year/mon/.../yday.

Changelog v1->v2:
1: Fix "no tabs" error caused by email client.
2: Remove explicit 'inline' in __isleap()
3: Move DIV() and LEAPS_THRU_END_OF(y) into regular C function along with
comment
4: Move gmtime() and localtime() into header file.

Thanks
Zhaolei

2009-07-20 09:55:30

by Zhao Lei

[permalink] [raw]
Subject: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

There are many similar code in kernel for one object:
convert time between calendar time and broken-down time.

Here is some source I found:
fs/ncpfs/dir.c
fs/smbfs/proc.c
fs/fat/misc.c
fs/udf/udftime.c
fs/cifs/netmisc.c
net/netfilter/xt_time.c
drivers/scsi/ips.c
drivers/input/misc/hp_sdc_rtc.c
drivers/rtc/rtc-lib.c
arch/ia64/hp/sim/boot/fw-emu.c
arch/m68k/mac/misc.c
arch/powerpc/kernel/time.c
arch/parisc/include/asm/rtc.h
...

We can make a common function for this type of convert,
At least we can get following benefit:
1: Make kernel simple and unify
2: Easy to fix bug in converting code
3: Reduce clone of code in future
For example, I'm trying to make ftrace display walltime,
this patch will make me easy.

This code is based on code from glibc-2.6

Signed-off-by: Zhao Lei <[email protected]>
---
include/linux/time.h | 66 +++++++++++++++++++++++
kernel/time/Makefile | 2 +-
kernel/time/timeconv.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 205 insertions(+), 1 deletions(-)
create mode 100644 kernel/time/timeconv.c

diff --git a/include/linux/time.h b/include/linux/time.h
index ea16c1a..d74bc0c 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -151,6 +151,72 @@ extern void update_xtime_cache(u64 nsec);
struct tms;
extern void do_sys_times(struct tms *);

+/*
+ * Similar to the struct tm in userspace <time.h>, but it needs to be here so
+ * that the kernel source is self contained.
+ */
+struct tm {
+ /*
+ * the number of seconds after the minute, normally in the range
+ * 0 to 59, but can be up to 60 to allow for leap seconds
+ */
+ int tm_sec;
+ /* the number of minutes after the hour, in the range 0 to 59*/
+ int tm_min;
+ /* the number of hours past midnight, in the range 0 to 23 */
+ int tm_hour;
+ /* the day of the month, in the range 1 to 31 */
+ int tm_mday;
+ /* the number of months since January, in the range 0 to 11 */
+ int tm_mon;
+ /* the number of years since 1900 */
+ int tm_year;
+ /* the number of days since Sunday, in the range 0 to 6 */
+ int tm_wday;
+ /* the number of days since January 1, in the range 0 to 365 */
+ int tm_yday;
+};
+
+extern struct tm *__offtime(__kernel_time_t totalsecs, int offset,
+ struct tm *result);
+
+/**
+ * gmtime_r - converts the calendar time to UTC broken-down time
+ *
+ * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
+ * Coordinated Universal Time (UTC).
+ * @result pointer to struct tm variable to receive broken-down time
+ *
+ * Return a pointer to the broken-down time result on success,
+ * and NULL on when the year does not fit into an integer.
+ *
+ * Similar to the gmtime_r() in glibc, broken-down time is expressed in
+ * Coordinated Universal Time (UTC).
+ */
+static inline struct tm *gmtime_r(__kernel_time_t totalsecs, struct tm *result)
+{
+ return __offtime(totalsecs, 0, result);
+}
+
+/**
+ * localtime_r - converts the calendar time to local broken-down time
+ *
+ * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
+ * Coordinated Universal Time (UTC).
+ * @result pointer to struct tm variable to receive broken-down time
+ *
+ * Return a pointer to the broken-down time result on success,
+ * and NULL on when the year does not fit into an integer.
+ *
+ * Similar to the localtime_r() in glibc, broken-down time is expressed
+ * relative to sys_tz.
+ */
+static inline struct tm *localtime_r(__kernel_time_t totalsecs,
+ struct tm *result)
+{
+ return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
+}
+
/**
* timespec_to_ns - Convert timespec to nanoseconds
* @ts: pointer to the timespec variable to be converted
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 0b0a636..ee26662 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,4 +1,4 @@
-obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o
+obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o timeconv.o

obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD) += clockevents.o
obj-$(CONFIG_GENERIC_CLOCKEVENTS) += tick-common.o
diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
new file mode 100644
index 0000000..5a76581
--- /dev/null
+++ b/kernel/time/timeconv.c
@@ -0,0 +1,138 @@
+/*
+ * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, Inc.
+ * This file is part of the GNU C Library.
+ * Contributed by Paul Eggert ([email protected]).
+ *
+ * The GNU C Library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * The GNU C Library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with the GNU C Library; see the file COPYING.LIB. If not,
+ * write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+/*
+ * Converts the calendar time to broken-down time representation
+ * Based on code from glibc-2.6
+ *
+ * 2009-7-14:
+ * Moved from glibc-2.6 to kernel by Zhaolei<[email protected]>
+ */
+
+#include <linux/time.h>
+#include <linux/module.h>
+
+/*
+ * Nonzero if YEAR is a leap year (every 4 years,
+ * except every 100th isn't, and every 400th is).
+ */
+static int __isleap(unsigned int year)
+{
+ return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
+}
+
+/* do a mathdiv for long type */
+static long math_div(long a, long b)
+{
+ return a / b - (a % b < 0);
+}
+
+/* How many leap years between y1 and y2, y1 must less or equal to y2 */
+static long leaps_between(long y1, long y2)
+{
+ long leaps1 = math_div(y1 - 1, 4) - math_div(y1 - 1, 100)
+ + math_div(y1 - 1, 400);
+ long leaps2 = math_div(y2 - 1, 4) - math_div(y2 - 1, 100)
+ + math_div(y2 - 1, 400);
+ return leaps2 - leaps1;
+}
+
+/* How many days come before each month (0-12). */
+static const unsigned short __mon_yday[2][13] = {
+ /* Normal years. */
+ {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
+ /* Leap years. */
+ {0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
+};
+
+#define SECS_PER_HOUR (60 * 60)
+#define SECS_PER_DAY (SECS_PER_HOUR * 24)
+
+/**
+ * __offtime - converts the calendar time to local broken-down time
+ *
+ * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
+ * Coordinated Universal Time (UTC).
+ * @offset offset seconds adding to totalsecs.
+ * @result pointer to struct tm variable to receive broken-down time
+ *
+ * Return a pointer to the broken-down time result on success,
+ * and NULL on when the year does not fit into an integer.
+ *
+ * This function is for internal use, call gmtime_r() and localtime_r() instead.
+ */
+struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
+{
+ long days, rem, y;
+ const unsigned short *ip;
+
+ days = totalsecs / SECS_PER_DAY;
+ rem = totalsecs % SECS_PER_DAY;
+ rem += offset;
+ while (rem < 0) {
+ rem += SECS_PER_DAY;
+ --days;
+ }
+ while (rem >= SECS_PER_DAY) {
+ rem -= SECS_PER_DAY;
+ ++days;
+ }
+
+ result->tm_hour = rem / SECS_PER_HOUR;
+ rem %= SECS_PER_HOUR;
+ result->tm_min = rem / 60;
+ result->tm_sec = rem % 60;
+
+ /* January 1, 1970 was a Thursday. */
+ result->tm_wday = (4 + days) % 7;
+ if (result->tm_wday < 0)
+ result->tm_wday += 7;
+
+ y = 1970;
+
+ while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
+ /* Guess a corrected year, assuming 365 days per year. */
+ long yg = y + math_div(days, 365);
+
+ /* Adjust DAYS and Y to match the guessed year. */
+ days -= (yg - y) * 365 + leaps_between(y, yg);
+ y = yg;
+ }
+
+ result->tm_year = y - 1900;
+ if (result->tm_year != y - 1900) {
+ /* The year cannot be represented due to overflow. */
+ return NULL;
+ }
+
+ result->tm_yday = days;
+
+ ip = __mon_yday[__isleap(y)];
+ for (y = 11; days < ip[y]; y--)
+ continue;
+ days -= ip[y];
+
+ result->tm_mon = y;
+ result->tm_mday = days + 1;
+
+ return result;
+}
+EXPORT_SYMBOL(__offtime);
--
1.5.5.3

2009-07-20 09:56:45

by Zhao Lei

[permalink] [raw]
Subject: [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat()

It is not necessary to write custom code for convert calendar time
to broken-down time.
localtime()/gmtime() is more generic to do that.

Signed-off-by: Zhao Lei <[email protected]>
---
fs/fat/misc.c | 60 +++++++++++++++++---------------------------------------
1 files changed, 18 insertions(+), 42 deletions(-)

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index a6c2047..3b9a9d0 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -9,6 +9,7 @@
#include <linux/module.h>
#include <linux/fs.h>
#include <linux/buffer_head.h>
+#include <linux/time.h>
#include "fat.h"

/*
@@ -155,10 +156,6 @@ extern struct timezone sys_tz;
#define SECS_PER_MIN 60
#define SECS_PER_HOUR (60 * 60)
#define SECS_PER_DAY (SECS_PER_HOUR * 24)
-#define UNIX_SECS_1980 315532800L
-#if BITS_PER_LONG == 64
-#define UNIX_SECS_2108 4354819200L
-#endif
/* days between 1.1.70 and 1.1.80 (2 leap days) */
#define DAYS_DELTA (365 * 10 + 2)
/* 120 (2100 - 1980) isn't leap year */
@@ -211,58 +208,38 @@ void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
__le16 *time, __le16 *date, u8 *time_cs)
{
- time_t second = ts->tv_sec;
- time_t day, leap_day, month, year;
-
- if (!sbi->options.tz_utc)
- second -= sys_tz.tz_minuteswest * SECS_PER_MIN;
+ struct tm tm;
+ if (sbi->options.tz_utc) {
+ gmtime_r(ts->tv_sec, &tm);
+ } else {
+ localtime_r(ts->tv_sec, &tm);
+ }

- /* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
- if (second < UNIX_SECS_1980) {
+ /* FAT can only support year between 1980 to 2107 */
+ if (tm.tm_year < 1980 - 1900) {
*time = 0;
*date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
if (time_cs)
*time_cs = 0;
return;
}
-#if BITS_PER_LONG == 64
- if (second >= UNIX_SECS_2108) {
+ if (tm.tm_year > 2107 - 1900) {
*time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
*date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
if (time_cs)
*time_cs = 199;
return;
}
-#endif

- day = second / SECS_PER_DAY - DAYS_DELTA;
- year = day / 365;
- leap_day = (year + 3) / 4;
- if (year > YEAR_2100) /* 2100 isn't leap year */
- leap_day--;
- if (year * 365 + leap_day > day)
- year--;
- leap_day = (year + 3) / 4;
- if (year > YEAR_2100) /* 2100 isn't leap year */
- leap_day--;
- day -= year * 365 + leap_day;
+ /* from 1900 -> from 1980 */
+ tm.tm_year -= 80;
+ /* 0~11 -> 1~12 */
+ tm.tm_mon++;
+ /* 0~59 -> 0~29(2sec counts) */
+ tm.tm_sec >>= 1;

- if (IS_LEAP_YEAR(year) && day == days_in_year[3]) {
- month = 2;
- } else {
- if (IS_LEAP_YEAR(year) && day > days_in_year[3])
- day--;
- for (month = 1; month < 12; month++) {
- if (days_in_year[month + 1] > day)
- break;
- }
- }
- day -= days_in_year[month];
-
- *time = cpu_to_le16(((second / SECS_PER_HOUR) % 24) << 11
- | ((second / SECS_PER_MIN) % 60) << 5
- | (second % SECS_PER_MIN) >> 1);
- *date = cpu_to_le16((year << 9) | (month << 5) | (day + 1));
+ *time = cpu_to_le16(tm.tm_hour << 11 | tm.tm_min << 5 | tm.tm_sec);
+ *date = cpu_to_le16(tm.tm_year << 9 | tm.tm_mon << 5 | tm.tm_mday);
if (time_cs)
*time_cs = (ts->tv_sec & 1) * 100 + ts->tv_nsec / 10000000;
}
@@ -283,4 +260,3 @@ int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
}
return err;
}
-
--
1.5.5.3

2009-07-20 10:04:07

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat()

On Mon 2009-07-20 17:57:22, Zhaolei wrote:
> It is not necessary to write custom code for convert calendar time
> to broken-down time.
> localtime()/gmtime() is more generic to do that.
>
> Signed-off-by: Zhao Lei <[email protected]>

Ack.

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-07-25 08:21:11

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

Zhaolei <[email protected]> writes:

> +/*
> + * Similar to the struct tm in userspace <time.h>, but it needs to be here so
> + * that the kernel source is self contained.
> + */
> +struct tm {
> + /*
> + * the number of seconds after the minute, normally in the range
> + * 0 to 59, but can be up to 60 to allow for leap seconds
> + */
> + int tm_sec;
> + /* the number of minutes after the hour, in the range 0 to 59*/
> + int tm_min;
> + /* the number of hours past midnight, in the range 0 to 23 */
> + int tm_hour;
> + /* the day of the month, in the range 1 to 31 */
> + int tm_mday;
> + /* the number of months since January, in the range 0 to 11 */
> + int tm_mon;
> + /* the number of years since 1900 */
> + int tm_year;

Why isn't this "long"? "int" can overflow.

> + /* the number of days since Sunday, in the range 0 to 6 */
> + int tm_wday;
> + /* the number of days since January 1, in the range 0 to 365 */
> + int tm_yday;
> +};

Those are needed?

> +
> +extern struct tm *__offtime(__kernel_time_t totalsecs, int offset,
> + struct tm *result);

Why isn't time_t simply, not __kernel_time_t?

> +/**
> + * gmtime_r - converts the calendar time to UTC broken-down time
> + *
> + * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
> + * Coordinated Universal Time (UTC).
> + * @result pointer to struct tm variable to receive broken-down time
> + *
> + * Return a pointer to the broken-down time result on success,
> + * and NULL on when the year does not fit into an integer.
> + *
> + * Similar to the gmtime_r() in glibc, broken-down time is expressed in
> + * Coordinated Universal Time (UTC).
> + */
> +static inline struct tm *gmtime_r(__kernel_time_t totalsecs, struct tm *result)
> +{
> + return __offtime(totalsecs, 0, result);
> +}
> +
> +/**
> + * localtime_r - converts the calendar time to local broken-down time
> + *
> + * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
> + * Coordinated Universal Time (UTC).
> + * @result pointer to struct tm variable to receive broken-down time
> + *
> + * Return a pointer to the broken-down time result on success,
> + * and NULL on when the year does not fit into an integer.
> + *
> + * Similar to the localtime_r() in glibc, broken-down time is expressed
> + * relative to sys_tz.
> + */
> +static inline struct tm *localtime_r(__kernel_time_t totalsecs,
> + struct tm *result)
> +{
> + return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
> +}

I think those are confusable. The real function of those needs to handle
timezone database. Especially, sys_tz.tz_minuteswest in localtime_r() is
known as wrong.

Are you going to fix it? Otherwise I don't think it would not be good to
use it easily as generic function like this.

> +/*
> + * Nonzero if YEAR is a leap year (every 4 years,
> + * except every 100th isn't, and every 400th is).
> + */
> +static int __isleap(unsigned int year)

long year. This breaks negative time_t.

> +{
> + return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
> +}

> +/**
> + * __offtime - converts the calendar time to local broken-down time
> + *
> + * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
> + * Coordinated Universal Time (UTC).
> + * @offset offset seconds adding to totalsecs.
> + * @result pointer to struct tm variable to receive broken-down time
> + *
> + * Return a pointer to the broken-down time result on success,
> + * and NULL on when the year does not fit into an integer.
> + *
> + * This function is for internal use, call gmtime_r() and localtime_r() instead.
> + */
> +struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
> +{

So, I suggest to consolidate this code only, and don't provide
gmtime_r()/localtime_r(), and use more good function name for
__offtime() (I'm not sure, however, personally I feel __offtime is not
obvious what's doing).

Thanks.
--
OGAWA Hirofumi <[email protected]>

2009-07-25 09:41:00

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat()

Zhaolei <[email protected]> writes:

> + if (sbi->options.tz_utc) {
> + gmtime_r(ts->tv_sec, &tm);
> + } else {
> + localtime_r(ts->tv_sec, &tm);
> + }

Missing error check.

> - /* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
> - if (second < UNIX_SECS_1980) {
> + /* FAT can only support year between 1980 to 2107 */
> + if (tm.tm_year < 1980 - 1900) {
> *time = 0;
> *date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
> if (time_cs)
> *time_cs = 0;
> return;
> }
> -#if BITS_PER_LONG == 64
> - if (second >= UNIX_SECS_2108) {
> + if (tm.tm_year > 2107 - 1900) {
> *time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
> *date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
> if (time_cs)
> *time_cs = 199;
> return;
> }
> -#endif

I think tm.tm_year is undefine in case of the error.

Thanks.
--
OGAWA Hirofumi <[email protected]>

2009-07-25 12:15:25

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

[resend: sorry if you got this twice]

OGAWA Hirofumi <[email protected]> writes:

> Are you going to fix it? Otherwise I don't think it would not be good to
> use it easily as generic function like this.

s/I don't think/I think/
--
OGAWA Hirofumi <[email protected]>

2009-07-25 16:36:00

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

OGAWA Hirofumi <[email protected]> writes:

> Are you going to fix it? Otherwise I don't think it would not be good to
> use it easily as generic function like this.

s/I don't think/I think/
--
OGAWA Hirofumi <[email protected]>

2009-07-27 03:12:47

by Zhao Lei

[permalink] [raw]
Subject: Re: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

OGAWA Hirofumi wrote:
> Zhaolei <[email protected]> writes:
>
>> +/*
>> + * Similar to the struct tm in userspace <time.h>, but it needs to be here so
>> + * that the kernel source is self contained.
>> + */
>> +struct tm {
>> + /*
>> + * the number of seconds after the minute, normally in the range
>> + * 0 to 59, but can be up to 60 to allow for leap seconds
>> + */
>> + int tm_sec;
>> + /* the number of minutes after the hour, in the range 0 to 59*/
>> + int tm_min;
>> + /* the number of hours past midnight, in the range 0 to 23 */
>> + int tm_hour;
>> + /* the day of the month, in the range 1 to 31 */
>> + int tm_mday;
>> + /* the number of months since January, in the range 0 to 11 */
>> + int tm_mon;
>> + /* the number of years since 1900 */
>> + int tm_year;
>
> Why isn't this "long"? "int" can overflow.

Hello, OGAWA-san:

Thanks for your review.

I selected int to make it same with user-space struct tm.

In 32-bit machine, long have same length with int, and still can overflow,
It we want to avoid it in all platform, we should use long long,
and make division complex.

Maybe make it same with user-space struct tm is ok.

>> + /* the number of days since Sunday, in the range 0 to 6 */
>> + int tm_wday;
>> + /* the number of days since January 1, in the range 0 to 365 */
>> + int tm_yday;
>> +};
>
> Those are needed?

Those are not needed by FAT-fs's code, but as a library,
I keep them for generic use and keep same with user-space struct tm.

>> +
>> +extern struct tm *__offtime(__kernel_time_t totalsecs, int offset,
>> + struct tm *result);
>
> Why isn't time_t simply, not __kernel_time_t?

Yes, thanks.
It should be time_t.
I will fix it.

>> +/**
>> + * gmtime_r - converts the calendar time to UTC broken-down time
>> + *
>> + * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
>> + * Coordinated Universal Time (UTC).
>> + * @result pointer to struct tm variable to receive broken-down time
>> + *
>> + * Return a pointer to the broken-down time result on success,
>> + * and NULL on when the year does not fit into an integer.
>> + *
>> + * Similar to the gmtime_r() in glibc, broken-down time is expressed in
>> + * Coordinated Universal Time (UTC).
>> + */
>> +static inline struct tm *gmtime_r(__kernel_time_t totalsecs, struct tm *result)
>> +{
>> + return __offtime(totalsecs, 0, result);
>> +}
>> +
>> +/**
>> + * localtime_r - converts the calendar time to local broken-down time
>> + *
>> + * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
>> + * Coordinated Universal Time (UTC).
>> + * @result pointer to struct tm variable to receive broken-down time
>> + *
>> + * Return a pointer to the broken-down time result on success,
>> + * and NULL on when the year does not fit into an integer.
>> + *
>> + * Similar to the localtime_r() in glibc, broken-down time is expressed
>> + * relative to sys_tz.
>> + */
>> +static inline struct tm *localtime_r(__kernel_time_t totalsecs,
>> + struct tm *result)
>> +{
>> + return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
>> +}
>
> I think those are confusable. The real function of those needs to handle
> timezone database. Especially, sys_tz.tz_minuteswest in localtime_r() is
> known as wrong.
>
> Are you going to fix it? Otherwise I don't think it would not be good to
> use it easily as generic function like this.

Actually, it is hard to select.

I don't schedule to introduce complex timezone database into kernel,
but as you said, localtime() without timezone database is not complete.
But localtime_r is easy to use and understood, it is similar with
userspace same-name function...

>> +/*
>> + * Nonzero if YEAR is a leap year (every 4 years,
>> + * except every 100th isn't, and every 400th is).
>> + */
>> +static int __isleap(unsigned int year)
>
> long year. This breaks negative time_t.
>
>> +{
>> + return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
>> +}
>
>> +/**
>> + * __offtime - converts the calendar time to local broken-down time
>> + *
>> + * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
>> + * Coordinated Universal Time (UTC).
>> + * @offset offset seconds adding to totalsecs.
>> + * @result pointer to struct tm variable to receive broken-down time
>> + *
>> + * Return a pointer to the broken-down time result on success,
>> + * and NULL on when the year does not fit into an integer.
>> + *
>> + * This function is for internal use, call gmtime_r() and localtime_r() instead.
>> + */
>> +struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
>> +{
>
> So, I suggest to consolidate this code only, and don't provide
> gmtime_r()/localtime_r(), and use more good function name for
> __offtime() (I'm not sure, however, personally I feel __offtime is not
> obvious what's doing).

What about just use gmtime_r(rename __offtime->gmtime_r)?

In fact I think both way(hode original localtime/gmtime or delete them) have
merit and demerit.
Hode them will make it easy to use, delete them will make function more exact.

Hi, Andrew
What is your opinion?

Thanks
Zhaolei

2009-07-27 03:18:26

by Zhao Lei

[permalink] [raw]
Subject: Re: Re: [PATCH v3 2/2] Use common localtime/gmtime in fat_time_unix2fat()

OGAWA Hirofumi wrote:
> Zhaolei <[email protected]> writes:
>
>> + if (sbi->options.tz_utc) {
>> + gmtime_r(ts->tv_sec, &tm);
>> + } else {
>> + localtime_r(ts->tv_sec, &tm);
>> + }
>
> Missing error check.
>
>> - /* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
>> - if (second < UNIX_SECS_1980) {
>> + /* FAT can only support year between 1980 to 2107 */
>> + if (tm.tm_year < 1980 - 1900) {
>> *time = 0;
>> *date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
>> if (time_cs)
>> *time_cs = 0;
>> return;
>> }
>> -#if BITS_PER_LONG == 64
>> - if (second >= UNIX_SECS_2108) {
>> + if (tm.tm_year > 2107 - 1900) {
>> *time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
>> *date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
>> if (time_cs)
>> *time_cs = 199;
>> return;
>> }
>> -#endif
>
> I think tm.tm_year is undefine in case of the error.

Hello, Ogawa-san:

Thanks for your review.
I'll fix them

Thanks
Zhaolei

2009-07-27 06:04:29

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

Zhaolei <[email protected]> writes:

>>> + /* the number of years since 1900 */
>>> + int tm_year;
>>
>> Why isn't this "long"? "int" can overflow.
>
> I selected int to make it same with user-space struct tm.
>
> In 32-bit machine, long have same length with int, and still can overflow,
> It we want to avoid it in all platform, we should use long long,
> and make division complex.
>
> Maybe make it same with user-space struct tm is ok.

time_t is also "long" on 32-bit machine, it does overflow?

>>> + /* the number of days since Sunday, in the range 0 to 6 */
>>> + int tm_wday;
>>> + /* the number of days since January 1, in the range 0 to 365 */
>>> + int tm_yday;
>>> +};
>>
>> Those are needed?
>
> Those are not needed by FAT-fs's code, but as a library,
> I keep them for generic use and keep same with user-space struct tm.

Yes. But, if there is no users, why need? I guess it makes slow thing,
and it is slow than FAT's one (because FAT's one is optimized for
FAT's timestamp range more or less).

>>> +static inline struct tm *localtime_r(__kernel_time_t totalsecs,
>>> + struct tm *result)
>>> +{
>>> + return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
>>> +}
>>
>> I think those are confusable. The real function of those needs to handle
>> timezone database. Especially, sys_tz.tz_minuteswest in localtime_r() is
>> known as wrong.
>>
>> Are you going to fix it? Otherwise I don't think it would not be good to
>> use it easily as generic function like this.
>
> Actually, it is hard to select.
>
> I don't schedule to introduce complex timezone database into kernel,
> but as you said, localtime() without timezone database is not complete.
> But localtime_r is easy to use and understood, it is similar with
> userspace same-name function...

Actually it is both (gmtime() is also needed to handle timezone).

I worry someone uses it and display/export to userland. After that, the
user will report the bug of it.

>>> +/*
>>> + * Nonzero if YEAR is a leap year (every 4 years,
>>> + * except every 100th isn't, and every 400th is).
>>> + */
>>> +static int __isleap(unsigned int year)
>>
>> long year. This breaks negative time_t.

Please "long year".

>>> +struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
>>> +{
>>
>> So, I suggest to consolidate this code only, and don't provide
>> gmtime_r()/localtime_r(), and use more good function name for
>> __offtime() (I'm not sure, however, personally I feel __offtime is not
>> obvious what's doing).
>
> What about just use gmtime_r(rename __offtime->gmtime_r)?

gmtime() also need to handle timezone actually.

> In fact I think both way(hode original localtime/gmtime or delete them) have
> merit and demerit.
> Hode them will make it easy to use, delete them will make function more exact.

Yes. But, how do you handle bug report?

Thanks.
--
OGAWA Hirofumi <[email protected]>

2009-07-27 22:44:43

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

On Sat 2009-07-25 14:42:06, OGAWA Hirofumi wrote:
> Zhaolei <[email protected]> writes:
>
> > +/*
> > + * Similar to the struct tm in userspace <time.h>, but it needs to be here so
> > + * that the kernel source is self contained.
> > + */
> > +struct tm {
> > + /*
> > + * the number of seconds after the minute, normally in the range
> > + * 0 to 59, but can be up to 60 to allow for leap seconds
> > + */
> > + int tm_sec;
> > + /* the number of minutes after the hour, in the range 0 to 59*/
> > + int tm_min;
> > + /* the number of hours past midnight, in the range 0 to 23 */
> > + int tm_hour;
> > + /* the day of the month, in the range 1 to 31 */
> > + int tm_mday;
> > + /* the number of months since January, in the range 0 to 11 */
> > + int tm_mon;
> > + /* the number of years since 1900 */
> > + int tm_year;
>
> Why isn't this "long"? "int" can overflow.

Overflow? Year?

I'm not sure how far ahead you are looking, but support for year
2000000000 (2e9) seems perfectly okay to me. (Our friendly star here,
sun, is going to die at cca year 5e9, for comparison. )

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2009-07-28 03:02:24

by Zhao Lei

[permalink] [raw]
Subject: Re: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

OGAWA Hirofumi wrote:
> Zhaolei <[email protected]> writes:
>
>>>> + /* the number of years since 1900 */
>>>> + int tm_year;
>>> Why isn't this "long"? "int" can overflow.
>> I selected int to make it same with user-space struct tm.
>>
>> In 32-bit machine, long have same length with int, and still can overflow,
>> It we want to avoid it in all platform, we should use long long,
>> and make division complex.
>>
>> Maybe make it same with user-space struct tm is ok.
>
> time_t is also "long" on 32-bit machine, it does overflow?

Hello, Ogawa-san:
CC: Andrew, ingo:

Yes, time_t on 32-bit machine will overflow on year-2038.
So tm.tm_year will NEVER overflow on 32-bit machine.

And in 64-bit machine, int type of tm.tm_year will overflow in
year 2,147,485,548, Is that enough?

I also like your suggestion to use long for year, so gmtime/localtime
will never return false on both 32 and 64 bit machine.

(btw, I don't understand why time_t is long, it have different length(limit)
in different arch, and make disunity)

>>>> + /* the number of days since Sunday, in the range 0 to 6 */
>>>> + int tm_wday;
>>>> + /* the number of days since January 1, in the range 0 to 365 */
>>>> + int tm_yday;
>>>> +};
>>> Those are needed?
>> Those are not needed by FAT-fs's code, but as a library,
>> I keep them for generic use and keep same with user-space struct tm.
>
> Yes. But, if there is no users, why need?
> I guess it makes slow thing, and it is slow than FAT's one
> (because FAT's one is optimized for FAT's timestamp range more or less).

There is no users of printk("%pf"), why not remove this?
And at least, I found one: drivers/char/efirtc.c
If I continue searching, maybe more.

>>>> +static inline struct tm *localtime_r(__kernel_time_t totalsecs,
>>>> + struct tm *result)
>>>> +{
>>>> + return __offtime(totalsecs, -sys_tz.tz_minuteswest * 60, result);
>>>> +}
>>> I think those are confusable. The real function of those needs to handle
>>> timezone database. Especially, sys_tz.tz_minuteswest in localtime_r() is
>>> known as wrong.
>>>
>>> Are you going to fix it? Otherwise I don't think it would not be good to
>>> use it easily as generic function like this.
>> Actually, it is hard to select.
>>
>> I don't schedule to introduce complex timezone database into kernel,
>> but as you said, localtime() without timezone database is not complete.
>> But localtime_r is easy to use and understood, it is similar with
>> userspace same-name function...
>
> Actually it is both (gmtime() is also needed to handle timezone).
>
> I worry someone uses it and display/export to userland. After that, the
> user will report the bug of it.

IHMO, user of these functions should understand what these functions is and
use these functions for right way.
If we give user a cdrom, it is user's responsibility not using is as cup stand.

At least, there function can make following source a fairy:
fs/ncpfs/dir.c
fs/smbfs/proc.c
fs/fat/misc.c
fs/udf/udftime.c
fs/cifs/netmisc.c
net/netfilter/xt_time.c
drivers/scsi/ips.c
...

It is different with user-land just because it lakes of large-complex locale
database.

>>>> +/*
>>>> + * Nonzero if YEAR is a leap year (every 4 years,
>>>> + * except every 100th isn't, and every 400th is).
>>>> + */
>>>> +static int __isleap(unsigned int year)
>>> long year. This breaks negative time_t.
>
> Please "long year".

Ok, or int year(after we get conclusion of long/int).

>>>> +struct tm *__offtime(__kernel_time_t totalsecs, int offset, struct tm *result)
>>>> +{
>>> So, I suggest to consolidate this code only, and don't provide
>>> gmtime_r()/localtime_r(), and use more good function name for
>>> __offtime() (I'm not sure, however, personally I feel __offtime is not
>>> obvious what's doing).
>> What about just use gmtime_r(rename __offtime->gmtime_r)?
>
> gmtime() also need to handle timezone actually.

So, maybe another function name as unmktime?

>> In fact I think both way(hode original localtime/gmtime or delete them) have
>> merit and demerit.
>> Hode them will make it easy to use, delete them will make function more exact.
>
> Yes. But, how do you handle bug report?

I will thanks you for attention and reporting first, then discuss on LKML
about detail of this problem, and get a conclusion and fix.

Now we are in step2: discussing.
We need to get a best way to fix this to make users happy.

Thanks
Zhaolei

2009-07-28 04:53:03

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

Pavel Machek <[email protected]> writes:

>> Why isn't this "long"? "int" can overflow.
>
> Overflow? Year?
>
> I'm not sure how far ahead you are looking, but support for year
> 2000000000 (2e9) seems perfectly okay to me. (Our friendly star here,
> sun, is going to die at cca year 5e9, for comparison. )

Yes. If source is guaranteed as only xtime, but I'd like to think all
case. Especially, because this is library.

Thanks.
--
OGAWA Hirofumi <[email protected]>

2009-07-28 05:12:28

by OGAWA Hirofumi

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] Add function to convert between calendar time and broken-down time for universal use

Zhaolei <[email protected]> writes:

> There is no users of printk("%pf"), why not remove this?

IMO, because it doesn't make slow thing.

> And at least, I found one: drivers/char/efirtc.c
> If I continue searching, maybe more.

If there is a user, I think it's ok.

> IHMO, user of these functions should understand what these functions is and
> use these functions for right way.
> If we give user a cdrom, it is user's responsibility not using is as cup stand.
>
> At least, there function can make following source a fairy:
> fs/ncpfs/dir.c
> fs/smbfs/proc.c
> fs/fat/misc.c
> fs/udf/udftime.c
> fs/cifs/netmisc.c
> net/netfilter/xt_time.c
> drivers/scsi/ips.c
> ...
>
> It is different with user-land just because it lakes of large-complex locale
> database.

Yes, and I'm understanding FAT has the bug, and there is actual bug
report (bug FAT requires this as on-disk format). However, my point is
why we increase the known bugs.

>> gmtime() also need to handle timezone actually.
>
> So, maybe another function name as unmktime?

Maybe. Um..., or time_to_tm()? I'm not sure.

Thanks.
--
OGAWA Hirofumi <[email protected]>

2009-07-30 05:36:31

by Zhao Lei

[permalink] [raw]
Subject: [PATCH v4 0/2] Add function to convert between calendar time and broken-down time for universal use

Hello, Andrew

This is Patch v4.
I updated this patch bases on Ogawa-san's suggestion.

Changelog v3->v4:
1: Use long instead of int for tm.tm_year to avoid overflow
2: Remove localtime() and gmtime() because they can't support timezone
database as user-more. And rename __offtime() to time_to_tm()
3: Use time_t instead of __kernel_time_t as function's argument type
4: __isleap(unsigned int year)->__isleap(long year) to avoid overflow

Changelog v2->v3:
Using a struct to save year/mon/.../yday.

Changelog v1->v2:
1: Fix "no tabs" error caused by email client.
2: Remove explicit 'inline' in __isleap()
3: Move DIV() and LEAPS_THRU_END_OF(y) into regular C function along with
comment
4: Move gmtime() and localtime() into header file.

Thanks
Zhaolei

2009-07-30 05:37:38

by Zhao Lei

[permalink] [raw]
Subject: [PATCH v4 1/2] Add function to convert between calendar time and broken-down time for universal use

There are many similar code in kernel for one object:
convert time between calendar time and broken-down time.

Here is some source I found:
fs/ncpfs/dir.c
fs/smbfs/proc.c
fs/fat/misc.c
fs/udf/udftime.c
fs/cifs/netmisc.c
net/netfilter/xt_time.c
drivers/scsi/ips.c
drivers/input/misc/hp_sdc_rtc.c
drivers/rtc/rtc-lib.c
arch/ia64/hp/sim/boot/fw-emu.c
arch/m68k/mac/misc.c
arch/powerpc/kernel/time.c
arch/parisc/include/asm/rtc.h
...

We can make a common function for this type of convert,
At least we can get following benefit:
1: Make kernel simple and unify
2: Easy to fix bug in converting code
3: Reduce clone of code in future
For example, I'm trying to make ftrace display walltime,
this patch will make me easy.

This code is based on code from glibc-2.6

Signed-off-by: Zhao Lei <[email protected]>
---
include/linux/time.h | 28 +++++++++++
kernel/time/Makefile | 2 +-
kernel/time/timeconv.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 156 insertions(+), 1 deletions(-)
create mode 100644 kernel/time/timeconv.c

diff --git a/include/linux/time.h b/include/linux/time.h
index ea16c1a..4bc7a91 100644
--- a/include/linux/time.h
+++ b/include/linux/time.h
@@ -151,6 +151,34 @@ extern void update_xtime_cache(u64 nsec);
struct tms;
extern void do_sys_times(struct tms *);

+/*
+ * Similar to the struct tm in userspace <time.h>, but it needs to be here so
+ * that the kernel source is self contained.
+ */
+struct tm {
+ /*
+ * the number of seconds after the minute, normally in the range
+ * 0 to 59, but can be up to 60 to allow for leap seconds
+ */
+ int tm_sec;
+ /* the number of minutes after the hour, in the range 0 to 59*/
+ int tm_min;
+ /* the number of hours past midnight, in the range 0 to 23 */
+ int tm_hour;
+ /* the day of the month, in the range 1 to 31 */
+ int tm_mday;
+ /* the number of months since January, in the range 0 to 11 */
+ int tm_mon;
+ /* the number of years since 1900 */
+ long tm_year;
+ /* the number of days since Sunday, in the range 0 to 6 */
+ int tm_wday;
+ /* the number of days since January 1, in the range 0 to 365 */
+ int tm_yday;
+};
+
+void time_to_tm(time_t totalsecs, int offset, struct tm *result);
+
/**
* timespec_to_ns - Convert timespec to nanoseconds
* @ts: pointer to the timespec variable to be converted
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index 0b0a636..ee26662 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -1,4 +1,4 @@
-obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o
+obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o timecompare.o timeconv.o

obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD) += clockevents.o
obj-$(CONFIG_GENERIC_CLOCKEVENTS) += tick-common.o
diff --git a/kernel/time/timeconv.c b/kernel/time/timeconv.c
new file mode 100644
index 0000000..86628e7
--- /dev/null
+++ b/kernel/time/timeconv.c
@@ -0,0 +1,127 @@
+/*
+ * Copyright (C) 1993, 1994, 1995, 1996, 1997 Free Software Foundation, Inc.
+ * This file is part of the GNU C Library.
+ * Contributed by Paul Eggert ([email protected]).
+ *
+ * The GNU C Library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * The GNU C Library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public
+ * License along with the GNU C Library; see the file COPYING.LIB. If not,
+ * write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+/*
+ * Converts the calendar time to broken-down time representation
+ * Based on code from glibc-2.6
+ *
+ * 2009-7-14:
+ * Moved from glibc-2.6 to kernel by Zhaolei<[email protected]>
+ */
+
+#include <linux/time.h>
+#include <linux/module.h>
+
+/*
+ * Nonzero if YEAR is a leap year (every 4 years,
+ * except every 100th isn't, and every 400th is).
+ */
+static int __isleap(long year)
+{
+ return (year) % 4 == 0 && ((year) % 100 != 0 || (year) % 400 == 0);
+}
+
+/* do a mathdiv for long type */
+static long math_div(long a, long b)
+{
+ return a / b - (a % b < 0);
+}
+
+/* How many leap years between y1 and y2, y1 must less or equal to y2 */
+static long leaps_between(long y1, long y2)
+{
+ long leaps1 = math_div(y1 - 1, 4) - math_div(y1 - 1, 100)
+ + math_div(y1 - 1, 400);
+ long leaps2 = math_div(y2 - 1, 4) - math_div(y2 - 1, 100)
+ + math_div(y2 - 1, 400);
+ return leaps2 - leaps1;
+}
+
+/* How many days come before each month (0-12). */
+static const unsigned short __mon_yday[2][13] = {
+ /* Normal years. */
+ {0, 31, 59, 90, 120, 151, 181, 212, 243, 273, 304, 334, 365},
+ /* Leap years. */
+ {0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366}
+};
+
+#define SECS_PER_HOUR (60 * 60)
+#define SECS_PER_DAY (SECS_PER_HOUR * 24)
+
+/**
+ * time_to_tm - converts the calendar time to local broken-down time
+ *
+ * @totalsecs the number of seconds elapsed since 00:00:00 on January 1, 1970,
+ * Coordinated Universal Time (UTC).
+ * @offset offset seconds adding to totalsecs.
+ * @result pointer to struct tm variable to receive broken-down time
+ */
+void time_to_tm(time_t totalsecs, int offset, struct tm *result)
+{
+ long days, rem, y;
+ const unsigned short *ip;
+
+ days = totalsecs / SECS_PER_DAY;
+ rem = totalsecs % SECS_PER_DAY;
+ rem += offset;
+ while (rem < 0) {
+ rem += SECS_PER_DAY;
+ --days;
+ }
+ while (rem >= SECS_PER_DAY) {
+ rem -= SECS_PER_DAY;
+ ++days;
+ }
+
+ result->tm_hour = rem / SECS_PER_HOUR;
+ rem %= SECS_PER_HOUR;
+ result->tm_min = rem / 60;
+ result->tm_sec = rem % 60;
+
+ /* January 1, 1970 was a Thursday. */
+ result->tm_wday = (4 + days) % 7;
+ if (result->tm_wday < 0)
+ result->tm_wday += 7;
+
+ y = 1970;
+
+ while (days < 0 || days >= (__isleap(y) ? 366 : 365)) {
+ /* Guess a corrected year, assuming 365 days per year. */
+ long yg = y + math_div(days, 365);
+
+ /* Adjust DAYS and Y to match the guessed year. */
+ days -= (yg - y) * 365 + leaps_between(y, yg);
+ y = yg;
+ }
+
+ result->tm_year = y - 1900;
+
+ result->tm_yday = days;
+
+ ip = __mon_yday[__isleap(y)];
+ for (y = 11; days < ip[y]; y--)
+ continue;
+ days -= ip[y];
+
+ result->tm_mon = y;
+ result->tm_mday = days + 1;
+}
+EXPORT_SYMBOL(time_to_tm);
--
1.5.5.3

2009-07-30 05:38:30

by Zhao Lei

[permalink] [raw]
Subject: [PATCH v4 2/2] Use common time_to_tm in fat_time_unix2fat()

It is not necessary to write custom code for convert calendar time
to broken-down time.
time_to_tm() is more generic to do that.

Signed-off-by: Zhao Lei <[email protected]>
---
fs/fat/misc.c | 57 +++++++++++++++------------------------------------------
1 files changed, 15 insertions(+), 42 deletions(-)

diff --git a/fs/fat/misc.c b/fs/fat/misc.c
index a6c2047..03bf98d 100644
--- a/fs/fat/misc.c
+++ b/fs/fat/misc.c
@@ -9,6 +9,7 @@
#include <linux/module.h>
#include <linux/fs.h>
#include <linux/buffer_head.h>
+#include <linux/time.h>
#include "fat.h"

/*
@@ -155,10 +156,6 @@ extern struct timezone sys_tz;
#define SECS_PER_MIN 60
#define SECS_PER_HOUR (60 * 60)
#define SECS_PER_DAY (SECS_PER_HOUR * 24)
-#define UNIX_SECS_1980 315532800L
-#if BITS_PER_LONG == 64
-#define UNIX_SECS_2108 4354819200L
-#endif
/* days between 1.1.70 and 1.1.80 (2 leap days) */
#define DAYS_DELTA (365 * 10 + 2)
/* 120 (2100 - 1980) isn't leap year */
@@ -211,58 +208,35 @@ void fat_time_fat2unix(struct msdos_sb_info *sbi, struct timespec *ts,
void fat_time_unix2fat(struct msdos_sb_info *sbi, struct timespec *ts,
__le16 *time, __le16 *date, u8 *time_cs)
{
- time_t second = ts->tv_sec;
- time_t day, leap_day, month, year;
+ struct tm tm;
+ time_to_tm(ts->tv_sec, sbi->options.tz_utc ? 0 :
+ -sys_tz.tz_minuteswest * 60, &tm);

- if (!sbi->options.tz_utc)
- second -= sys_tz.tz_minuteswest * SECS_PER_MIN;
-
- /* Jan 1 GMT 00:00:00 1980. But what about another time zone? */
- if (second < UNIX_SECS_1980) {
+ /* FAT can only support year between 1980 to 2107 */
+ if (tm.tm_year < 1980 - 1900) {
*time = 0;
*date = cpu_to_le16((0 << 9) | (1 << 5) | 1);
if (time_cs)
*time_cs = 0;
return;
}
-#if BITS_PER_LONG == 64
- if (second >= UNIX_SECS_2108) {
+ if (tm.tm_year > 2107 - 1900) {
*time = cpu_to_le16((23 << 11) | (59 << 5) | 29);
*date = cpu_to_le16((127 << 9) | (12 << 5) | 31);
if (time_cs)
*time_cs = 199;
return;
}
-#endif

- day = second / SECS_PER_DAY - DAYS_DELTA;
- year = day / 365;
- leap_day = (year + 3) / 4;
- if (year > YEAR_2100) /* 2100 isn't leap year */
- leap_day--;
- if (year * 365 + leap_day > day)
- year--;
- leap_day = (year + 3) / 4;
- if (year > YEAR_2100) /* 2100 isn't leap year */
- leap_day--;
- day -= year * 365 + leap_day;
-
- if (IS_LEAP_YEAR(year) && day == days_in_year[3]) {
- month = 2;
- } else {
- if (IS_LEAP_YEAR(year) && day > days_in_year[3])
- day--;
- for (month = 1; month < 12; month++) {
- if (days_in_year[month + 1] > day)
- break;
- }
- }
- day -= days_in_year[month];
+ /* from 1900 -> from 1980 */
+ tm.tm_year -= 80;
+ /* 0~11 -> 1~12 */
+ tm.tm_mon++;
+ /* 0~59 -> 0~29(2sec counts) */
+ tm.tm_sec >>= 1;

- *time = cpu_to_le16(((second / SECS_PER_HOUR) % 24) << 11
- | ((second / SECS_PER_MIN) % 60) << 5
- | (second % SECS_PER_MIN) >> 1);
- *date = cpu_to_le16((year << 9) | (month << 5) | (day + 1));
+ *time = cpu_to_le16(tm.tm_hour << 11 | tm.tm_min << 5 | tm.tm_sec);
+ *date = cpu_to_le16(tm.tm_year << 9 | tm.tm_mon << 5 | tm.tm_mday);
if (time_cs)
*time_cs = (ts->tv_sec & 1) * 100 + ts->tv_nsec / 10000000;
}
@@ -283,4 +257,3 @@ int fat_sync_bhs(struct buffer_head **bhs, int nr_bhs)
}
return err;
}
-
--
1.5.5.3