| Message ID | YDNdAiOkEYOfmbhT@humpty.home.comstyle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: Remove time_t cast for OpenBSD | expand |
On 22/02/2021 08.28, Brad Smith wrote: > OpenBSD has supported 64-bit time_t across all archs since 5.5 released in 2014. > > Remove a time_t cast that is no longer necessary. > > > Signed-off-by: Brad Smith <brad@comstyle.com> > > diff --git a/migration/savevm.c b/migration/savevm.c > index 52e2d72e4b..9557f85ba9 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2849,8 +2849,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate, > if (name) { > pstrcpy(sn->name, sizeof(sn->name), name); > } else { > - /* cast below needed for OpenBSD where tv_sec is still 'long' */ > - localtime_r((const time_t *)&tv.tv_sec, &tm); > + localtime_r(&tv.tv_sec, &tm); > strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm); > } Please make sure to CC: the maintainers (see MAINTAINERS file). Done now. Reviewed-by: Thomas Huth <thuth@redhat.com>
Le 08/03/2021 à 12:46, Thomas Huth a écrit : > On 22/02/2021 08.28, Brad Smith wrote: >> OpenBSD has supported 64-bit time_t across all archs since 5.5 released in 2014. >> >> Remove a time_t cast that is no longer necessary. >> >> >> Signed-off-by: Brad Smith <brad@comstyle.com> >> >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 52e2d72e4b..9557f85ba9 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -2849,8 +2849,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate, >> if (name) { >> pstrcpy(sn->name, sizeof(sn->name), name); >> } else { >> - /* cast below needed for OpenBSD where tv_sec is still 'long' */ >> - localtime_r((const time_t *)&tv.tv_sec, &tm); >> + localtime_r(&tv.tv_sec, &tm); >> strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm); >> } > but the qemu_timeval from "include/sysemu/os-win32.h" still uses a long: is this file compiled for win32? Thanks, Laurent
On 3/8/2021 6:46 AM, Thomas Huth wrote: > On 22/02/2021 08.28, Brad Smith wrote: >> OpenBSD has supported 64-bit time_t across all archs since 5.5 >> released in 2014. >> >> Remove a time_t cast that is no longer necessary. >> >> >> Signed-off-by: Brad Smith <brad@comstyle.com> >> >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 52e2d72e4b..9557f85ba9 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -2849,8 +2849,7 @@ bool save_snapshot(const char *name, bool >> overwrite, const char *vmstate, >> if (name) { >> pstrcpy(sn->name, sizeof(sn->name), name); >> } else { >> - /* cast below needed for OpenBSD where tv_sec is >> still 'long' */ >> - localtime_r((const time_t *)&tv.tv_sec, &tm); >> + localtime_r(&tv.tv_sec, &tm); >> strftime(sn->name, sizeof(sn->name), >> "vm-%Y%m%d%H%M%S", &tm); >> } > > Please make sure to CC: the maintainers (see MAINTAINERS file). Done now. > > Reviewed-by: Thomas Huth <thuth@redhat.com> My bad. That was an oversight on my part. I was quick to send it to the list without thinking about something like that.
* Laurent Vivier (laurent@vivier.eu) wrote: > Le 08/03/2021 à 12:46, Thomas Huth a écrit : > > On 22/02/2021 08.28, Brad Smith wrote: > >> OpenBSD has supported 64-bit time_t across all archs since 5.5 released in 2014. > >> > >> Remove a time_t cast that is no longer necessary. > >> > >> > >> Signed-off-by: Brad Smith <brad@comstyle.com> > >> > >> diff --git a/migration/savevm.c b/migration/savevm.c > >> index 52e2d72e4b..9557f85ba9 100644 > >> --- a/migration/savevm.c > >> +++ b/migration/savevm.c > >> @@ -2849,8 +2849,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate, > >> if (name) { > >> pstrcpy(sn->name, sizeof(sn->name), name); > >> } else { > >> - /* cast below needed for OpenBSD where tv_sec is still 'long' */ > >> - localtime_r((const time_t *)&tv.tv_sec, &tm); > >> + localtime_r(&tv.tv_sec, &tm); > >> strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm); > >> } > > > > but the qemu_timeval from "include/sysemu/os-win32.h" still uses a long: is this file compiled for > win32? Yep this fails for me when built with x86_64-w64-mingw32- (it's fine with i686-w64-mingw32- ) Dave > Thanks, > Laurent >
On Thu, Mar 11, 2021 at 06:28:57PM +0000, Dr. David Alan Gilbert wrote: > * Laurent Vivier (laurent@vivier.eu) wrote: > > Le 08/03/2021 à 12:46, Thomas Huth a écrit : > > > On 22/02/2021 08.28, Brad Smith wrote: > > >> OpenBSD has supported 64-bit time_t across all archs since 5.5 released in 2014. > > >> > > >> Remove a time_t cast that is no longer necessary. > > >> > > >> > > >> Signed-off-by: Brad Smith <brad@comstyle.com> > > >> > > >> diff --git a/migration/savevm.c b/migration/savevm.c > > >> index 52e2d72e4b..9557f85ba9 100644 > > >> --- a/migration/savevm.c > > >> +++ b/migration/savevm.c > > >> @@ -2849,8 +2849,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate, > > >> if (name) { > > >> pstrcpy(sn->name, sizeof(sn->name), name); > > >> } else { > > >> - /* cast below needed for OpenBSD where tv_sec is still 'long' */ > > >> - localtime_r((const time_t *)&tv.tv_sec, &tm); > > >> + localtime_r(&tv.tv_sec, &tm); > > >> strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm); > > >> } > > > > > > > but the qemu_timeval from "include/sysemu/os-win32.h" still uses a long: is this file compiled for > > win32? > > Yep this fails for me when built with x86_64-w64-mingw32- (it's fine > with i686-w64-mingw32- ) We could just switch the code to use GDateTime from GLib and thus avoid portability issues. I think this should be equivalent: g_autoptr(GDateTime) now = g_date_time_new_now_local(); g_autofree char *nowstr = g_date_time_format(now, "vm-%Y%m%d%H%M%s"); strncpy(sn->name, sizeof(sn->name), nowstr); Regards, Daniel
On 3/11/2021 1:39 PM, Daniel P. Berrangé wrote: > On Thu, Mar 11, 2021 at 06:28:57PM +0000, Dr. David Alan Gilbert wrote: >> * Laurent Vivier (laurent@vivier.eu) wrote: >>> Le 08/03/2021 à 12:46, Thomas Huth a écrit : >>>> On 22/02/2021 08.28, Brad Smith wrote: >>>>> OpenBSD has supported 64-bit time_t across all archs since 5.5 released in 2014. >>>>> >>>>> Remove a time_t cast that is no longer necessary. >>>>> >>>>> >>>>> Signed-off-by: Brad Smith <brad@comstyle.com> >>>>> >>>>> diff --git a/migration/savevm.c b/migration/savevm.c >>>>> index 52e2d72e4b..9557f85ba9 100644 >>>>> --- a/migration/savevm.c >>>>> +++ b/migration/savevm.c >>>>> @@ -2849,8 +2849,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate, >>>>> if (name) { >>>>> pstrcpy(sn->name, sizeof(sn->name), name); >>>>> } else { >>>>> - /* cast below needed for OpenBSD where tv_sec is still 'long' */ >>>>> - localtime_r((const time_t *)&tv.tv_sec, &tm); >>>>> + localtime_r(&tv.tv_sec, &tm); >>>>> strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm); >>>>> } >>> but the qemu_timeval from "include/sysemu/os-win32.h" still uses a long: is this file compiled for >>> win32? >> Yep this fails for me when built with x86_64-w64-mingw32- (it's fine >> with i686-w64-mingw32- ) > We could just switch the code to use GDateTime from GLib and thus > avoid portability issues. I think this should be equivalent: > > g_autoptr(GDateTime) now = g_date_time_new_now_local(); > g_autofree char *nowstr = g_date_time_format(now, "vm-%Y%m%d%H%M%s"); > strncpy(sn->name, sizeof(sn->name), nowstr); Which way do you guys want to go? Something like above, remove the comment or some variation on the comment but not mentioning OpenBSD since it is no longer relevant?
On 3/13/2021 6:33 PM, Brad Smith wrote: > On 3/11/2021 1:39 PM, Daniel P. Berrangé wrote: >> On Thu, Mar 11, 2021 at 06:28:57PM +0000, Dr. David Alan Gilbert wrote: >>> * Laurent Vivier (laurent@vivier.eu) wrote: >>>> Le 08/03/2021 à 12:46, Thomas Huth a écrit : >>>>> On 22/02/2021 08.28, Brad Smith wrote: >>>>>> OpenBSD has supported 64-bit time_t across all archs since 5.5 >>>>>> released in 2014. >>>>>> >>>>>> Remove a time_t cast that is no longer necessary. >>>>>> >>>>>> >>>>>> Signed-off-by: Brad Smith <brad@comstyle.com> >>>>>> >>>>>> diff --git a/migration/savevm.c b/migration/savevm.c >>>>>> index 52e2d72e4b..9557f85ba9 100644 >>>>>> --- a/migration/savevm.c >>>>>> +++ b/migration/savevm.c >>>>>> @@ -2849,8 +2849,7 @@ bool save_snapshot(const char *name, bool >>>>>> overwrite, const char *vmstate, >>>>>> if (name) { >>>>>> pstrcpy(sn->name, sizeof(sn->name), name); >>>>>> } else { >>>>>> - /* cast below needed for OpenBSD where tv_sec is >>>>>> still 'long' */ >>>>>> - localtime_r((const time_t *)&tv.tv_sec, &tm); >>>>>> + localtime_r(&tv.tv_sec, &tm); >>>>>> strftime(sn->name, sizeof(sn->name), >>>>>> "vm-%Y%m%d%H%M%S", &tm); >>>>>> } >>>> but the qemu_timeval from "include/sysemu/os-win32.h" still uses a >>>> long: is this file compiled for >>>> win32? >>> Yep this fails for me when built with x86_64-w64-mingw32- (it's fine >>> with i686-w64-mingw32- ) >> We could just switch the code to use GDateTime from GLib and thus >> avoid portability issues. I think this should be equivalent: >> >> g_autoptr(GDateTime) now = g_date_time_new_now_local(); >> g_autofree char *nowstr = g_date_time_format(now, >> "vm-%Y%m%d%H%M%s"); >> strncpy(sn->name, sizeof(sn->name), nowstr); > > Which way do you guys want to go? Something like above, remove the > comment > or some variation on the comment but not mentioning OpenBSD since it > is no > longer relevant? Anyone?
On Wed, Mar 31, 2021 at 03:26:16PM -0400, Brad Smith wrote: > On 3/13/2021 6:33 PM, Brad Smith wrote: > > On 3/11/2021 1:39 PM, Daniel P. Berrangé wrote: > > > On Thu, Mar 11, 2021 at 06:28:57PM +0000, Dr. David Alan Gilbert wrote: > > > > * Laurent Vivier (laurent@vivier.eu) wrote: > > > > > Le 08/03/2021 à 12:46, Thomas Huth a écrit : > > > > > > On 22/02/2021 08.28, Brad Smith wrote: > > > > > > > OpenBSD has supported 64-bit time_t across all archs > > > > > > > since 5.5 released in 2014. > > > > > > > > > > > > > > Remove a time_t cast that is no longer necessary. > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Brad Smith <brad@comstyle.com> > > > > > > > > > > > > > > diff --git a/migration/savevm.c b/migration/savevm.c > > > > > > > index 52e2d72e4b..9557f85ba9 100644 > > > > > > > --- a/migration/savevm.c > > > > > > > +++ b/migration/savevm.c > > > > > > > @@ -2849,8 +2849,7 @@ bool save_snapshot(const char > > > > > > > *name, bool overwrite, const char *vmstate, > > > > > > > if (name) { > > > > > > > pstrcpy(sn->name, sizeof(sn->name), name); > > > > > > > } else { > > > > > > > - /* cast below needed for OpenBSD where > > > > > > > tv_sec is still 'long' */ > > > > > > > - localtime_r((const time_t *)&tv.tv_sec, &tm); > > > > > > > + localtime_r(&tv.tv_sec, &tm); > > > > > > > strftime(sn->name, sizeof(sn->name), > > > > > > > "vm-%Y%m%d%H%M%S", &tm); > > > > > > > } > > > > > but the qemu_timeval from "include/sysemu/os-win32.h" still > > > > > uses a long: is this file compiled for > > > > > win32? > > > > Yep this fails for me when built with x86_64-w64-mingw32- (it's fine > > > > with i686-w64-mingw32- ) > > > We could just switch the code to use GDateTime from GLib and thus > > > avoid portability issues. I think this should be equivalent: > > > > > > g_autoptr(GDateTime) now = g_date_time_new_now_local(); > > > g_autofree char *nowstr = g_date_time_format(now, > > > "vm-%Y%m%d%H%M%s"); > > > strncpy(sn->name, sizeof(sn->name), nowstr); > > > > Which way do you guys want to go? Something like above, remove the > > comment > > or some variation on the comment but not mentioning OpenBSD since it is > > no > > longer relevant? > > Anyone? Personally I always favour using GLib APIs if there's an applicable one, since it eliminates portability problems - or rather offloads them to the GLib maintainers, who have already solved them generally. Regards, Daniel
On 4/1/2021 4:14 AM, Daniel P. Berrangé wrote: > On Wed, Mar 31, 2021 at 03:26:16PM -0400, Brad Smith wrote: >> On 3/13/2021 6:33 PM, Brad Smith wrote: >>> On 3/11/2021 1:39 PM, Daniel P. Berrangé wrote: >>>> On Thu, Mar 11, 2021 at 06:28:57PM +0000, Dr. David Alan Gilbert wrote: >>>>> * Laurent Vivier (laurent@vivier.eu) wrote: >>>>>> Le 08/03/2021 à 12:46, Thomas Huth a écrit : >>>>>>> On 22/02/2021 08.28, Brad Smith wrote: >>>>>>>> OpenBSD has supported 64-bit time_t across all archs >>>>>>>> since 5.5 released in 2014. >>>>>>>> >>>>>>>> Remove a time_t cast that is no longer necessary. >>>>>>>> >>>>>>>> >>>>>>>> Signed-off-by: Brad Smith <brad@comstyle.com> >>>>>>>> >>>>>>>> diff --git a/migration/savevm.c b/migration/savevm.c >>>>>>>> index 52e2d72e4b..9557f85ba9 100644 >>>>>>>> --- a/migration/savevm.c >>>>>>>> +++ b/migration/savevm.c >>>>>>>> @@ -2849,8 +2849,7 @@ bool save_snapshot(const char >>>>>>>> *name, bool overwrite, const char *vmstate, >>>>>>>> if (name) { >>>>>>>> pstrcpy(sn->name, sizeof(sn->name), name); >>>>>>>> } else { >>>>>>>> - /* cast below needed for OpenBSD where >>>>>>>> tv_sec is still 'long' */ >>>>>>>> - localtime_r((const time_t *)&tv.tv_sec, &tm); >>>>>>>> + localtime_r(&tv.tv_sec, &tm); >>>>>>>> strftime(sn->name, sizeof(sn->name), >>>>>>>> "vm-%Y%m%d%H%M%S", &tm); >>>>>>>> } >>>>>> but the qemu_timeval from "include/sysemu/os-win32.h" still >>>>>> uses a long: is this file compiled for >>>>>> win32? >>>>> Yep this fails for me when built with x86_64-w64-mingw32- (it's fine >>>>> with i686-w64-mingw32- ) >>>> We could just switch the code to use GDateTime from GLib and thus >>>> avoid portability issues. I think this should be equivalent: >>>> >>>> g_autoptr(GDateTime) now = g_date_time_new_now_local(); >>>> g_autofree char *nowstr = g_date_time_format(now, >>>> "vm-%Y%m%d%H%M%s"); >>>> strncpy(sn->name, sizeof(sn->name), nowstr); >>> Which way do you guys want to go? Something like above, remove the >>> comment >>> or some variation on the comment but not mentioning OpenBSD since it is >>> no >>> longer relevant? >> Anyone? > Personally I always favour using GLib APIs if there's an applicable one, > since it eliminates portability problems - or rather offloads them to > the GLib maintainers, who have already solved them generally. Can you please submit a proper diff and I'll drop this?
diff --git a/migration/savevm.c b/migration/savevm.c index 52e2d72e4b..9557f85ba9 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2849,8 +2849,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate, if (name) { pstrcpy(sn->name, sizeof(sn->name), name); } else { - /* cast below needed for OpenBSD where tv_sec is still 'long' */ - localtime_r((const time_t *)&tv.tv_sec, &tm); + localtime_r(&tv.tv_sec, &tm); strftime(sn->name, sizeof(sn->name), "vm-%Y%m%d%H%M%S", &tm); }
OpenBSD has supported 64-bit time_t across all archs since 5.5 released in 2014. Remove a time_t cast that is no longer necessary. Signed-off-by: Brad Smith <brad@comstyle.com>