Re: [Y2038] [PATCH 0/3] introduce new evdev interface type

[Arnd Bergmann]

On Thursday 03 December 2015 20:49:06 Pingbo Wen wrote:
> > å 2015å12æ1æï18:47ïArnd Bergmann <arnd@xxxxxxxx> åéï
> > On Tuesday 01 December 2015 16:34:00 Pingbo Wen wrote:
> >> We can force kernel using monotonic time in EV_IF_LEGACY interface, and
> >> making input_event independent from time_t(after evdev has converted to
> >> input_value, itâs easy to do that), but that also imply userspace
> >> must change their code to fit this change. If changing userspace code is
> >> a mandatory option, why not to force them do a complete conversion?
> > 
> > Most user space programs won't care, as they don't even look at the tv_sec
> > portion, and the goal is to avoid having to change them.
> > 
> > There is still an open question to how exactly we want to get user space
> > to change.
> > 
> > We could do some compile-time trick by having a union in struct input_event
> > and mark the existing timeval part as deprecated, so we flag any use of the
> > 32-bit tv_sec member, such as:
> > 
> > struct input_event {
> > #if !defined(__KERNEL__) && __TIME_T_BITS == __BITS_PER_LONG
> >        struct timeval time;
> 
> > #else
> > 	struct {
> > 		union {
> > 			__u32 tv_sec __attribute__((deprecated));
> > 			__u32 tv_sec_monotonic;
> > 		};
> > 		__s32 tv_usec;
> > 	} time;
> > #endif
> >        __u16 type;
> >        __u16 code;
> >        __s32 value;
> > };
> 
> I have one question here, if userspace use this structure, all helper functions
> of timeval will not work. And userspace need to write extra helper function for
> this fake timeval. This just create an another urgly time structure.

Correct, this is a useful side-effect of the change: any user space access to
the event->time member that assumes it's a timeval will cause a compile-time
warning or error (depending on the access), which helps us identify the
broken code and fix it to use monotonic times as well as access the right
struct members.

> And this method also forces most of old binaries to compile with new libc, adjust
> their codes with new fake time structure.
> 
> Besides, I get an idea to combine your structure with input_composite_event:
> 
> union {
> 	struct {
> 		__s32 tv_usec;
> 		__s32 tv_sec;
> 	};
> 	__s64 time;
> } time;
> 
> I prefer to use a single s64 timestamp, if our goal is to remove timeval from kernel.

We can't really remove this instance of timeval anyway, so adding an __s64 member
here is not all that helpful. We should use __s64 nanoseconds for new interfaces
like this, but I see no reason to change the one we have here.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/