On Thursday 12 November 2015 19:03:41 Amitoj Kaur Chawla wrote: > 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 impacts the layout of the structure retrieving profile > data as it is being used in a vendor specific command that can get > sent from user space and thus requires change in the ioctl interface. > > Signed-off-by: Amitoj Kaur Chawla <[email protected]>
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; > _______________________________________________ Y2038 mailing list [email protected] https://lists.linaro.org/mailman/listinfo/y2038
