On Tue, Nov 17, 2015 at 2:28 AM, Arnd Bergmann arnd@arndb.de 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.