On Tue, Nov 17, 2015 at 2:28 AM, Arnd Bergmann <[email protected]> wrote: > I think in this case, while the change is not necessarily wrong > (it has no effect on the generated binary), we really want a code > comment at the point where the overflow happens. > > Aside from that, the description seems misleading: the patch does > not /solve/ impact the layout of the structure but instead hides the > fact that the code is still broken, by avoiding the build error > that we get for the incorrect code. > > What the changelog should get across here is that the maintainers > have to make a choice between two scenarios: > > a) accept this patch, and live with the fact that the driver will > silently break and return an incorrect value in io_profile_start_time > at some point in the future (2038 or 2106, depending on whether user > space interprets it as signed or unsigned). This may or may not > be acceptable. > > b) Not take this patch, and get a compile error at some point in > the future for any configuration that intentionally removes the > time_t definition in order to find broken code. They can then > add a new command that has an extended time format to fix the > issue properly. > >> diff --git a/drivers/scsi/bfa/bfa_fcpim.c b/drivers/scsi/bfa/bfa_fcpim.c >> index 6730340..e5c211f 100644 >> --- a/drivers/scsi/bfa/bfa_fcpim.c >> +++ b/drivers/scsi/bfa/bfa_fcpim.c >> @@ -1478,7 +1478,7 @@ bfa_itnim_get_ioprofile(struct bfa_itnim_s *itnim, >> return BFA_STATUS_IOPROFILE_OFF; >> >> itnim->ioprofile.index = BFA_IOBUCKET_MAX; >> - itnim->ioprofile.io_profile_start_time = (u32)(time_t) >> + itnim->ioprofile.io_profile_start_time = (u32) >> bfa_io_profile_start_time(itnim->bfa); >> itnim->ioprofile.clock_res_mul = bfa_io_lat_clock_res_mul; >> itnim->ioprofile.clock_res_div = bfa_io_lat_clock_res_div; >> >
So in addition to removing the cast I'm adding a comment where io_profile_start_time is declared that the variable would overflow in 2106 or 2038. Also the changelog would look something like: 32 bit systems using 'time_t' will break in the year 2038, so we modify the code appropriately. This patch removes the cast to 'time_t' in the assignment statement since we are eventually removing the time_t definition from the kernel as an effort to solve the y2038 problem. This change will avoid the build error but the code is still broken and requires a change in the ioctl interface. Further, a comment has been added to the declaration of io_profile_start_time since the variable will break in 2038 or 2106 depending on user space interpreting it as signed or unsigned. -- Amitoj _______________________________________________ Y2038 mailing list [email protected] https://lists.linaro.org/mailman/listinfo/y2038
