| Message ID | 20200915121318.247-5-luoyonggang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | W32, W64 msys2/mingw patches | expand |
On Tue, Sep 15, 2020 at 08:12:56PM +0800, Yonggang Luo wrote: > First, this reduce the size of configure, configure are tending to removal in future, > and this didn't introduce any new feature or remove any exist feature. > Second, the current localtime_r detection are conflict with ncursesw detection in > mingw, when ncursesw detected, it will provide the following compile flags > pkg-config --cflags ncursesw > -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC:/CI-Tools/msys64/mingw64/include/ncursesw > And the compile flag _POSIX_C_SOURCE will always cause _POSIX_THREAD_SAFE_FUNCTIONS to > be defined, in new version of mingw, that's will cause localtime_r to be defined. > But the configure script didn't provide _POSIX_C_SOURCE macro, and that's will result > localtime_r not detected because localtime_r are defined in forceinline manner. ncursesw is just one of the three curses impls we can select for building against, so it doesn't feel right to make an assumption that _POSIX_C_SOURCE is always defined. > > And finally cause conflict between QEMU defined localtime_r > struct tm *localtime_r(const time_t *timep, struct tm *result); > with mingw defined localtime_r > > ``` > #if defined(_POSIX_C_SOURCE) && !defined(_POSIX_THREAD_SAFE_FUNCTIONS) > #define _POSIX_THREAD_SAFE_FUNCTIONS 200112L > #endif > > #ifdef _POSIX_THREAD_SAFE_FUNCTIONS > __forceinline struct tm *__CRTDECL localtime_r(const time_t *_Time, struct tm *_Tm) { > return localtime_s(_Tm, _Time) ? NULL : _Tm; > } > __forceinline struct tm *__CRTDECL gmtime_r(const time_t *_Time, struct tm *_Tm) { > return gmtime_s(_Tm, _Time) ? NULL : _Tm; > } > __forceinline char *__CRTDECL ctime_r(const time_t *_Time, char *_Str) { > return ctime_s(_Str, 0x7fffffff, _Time) ? NULL : _Str; > } > __forceinline char *__CRTDECL asctime_r(const struct tm *_Tm, char * _Str) { > return asctime_s(_Str, 0x7fffffff, _Tm) ? NULL : _Str; > } > #endif > ``` > > So I suggest remove this configure script, and restrict msys2/mingw version to easy to maintain. > And use _POSIX_THREAD_SAFE_FUNCTIONS to guard the localtime_r and counterpart functions > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com> > --- > configure | 34 ---------------------------------- > include/sysemu/os-win32.h | 4 ++-- > util/oslib-win32.c | 2 +- > 3 files changed, 3 insertions(+), 37 deletions(-) > > diff --git a/configure b/configure > index dc4b7a2e55..bac48b5b49 100755 > --- a/configure > +++ b/configure > @@ -2496,37 +2496,6 @@ if test "$vhost_net" = ""; then > test "$vhost_kernel" = "yes" && vhost_net=yes > fi > > -########################################## > -# MinGW / Mingw-w64 localtime_r/gmtime_r check > - > -if test "$mingw32" = "yes"; then > - # Some versions of MinGW / Mingw-w64 lack localtime_r > - # and gmtime_r entirely. > - # > - # Some versions of Mingw-w64 define a macro for > - # localtime_r/gmtime_r. > - # > - # Some versions of Mingw-w64 will define functions > - # for localtime_r/gmtime_r, but only if you have > - # _POSIX_THREAD_SAFE_FUNCTIONS defined. For fun > - # though, unistd.h and pthread.h both define > - # that for you. > - # > - # So this #undef localtime_r and #include <unistd.h> > - # are not in fact redundant. > -cat > $TMPC << EOF > -#include <unistd.h> > -#include <time.h> > -#undef localtime_r > -int main(void) { localtime_r(NULL, NULL); return 0; } > -EOF > - if compile_prog "" "" ; then > - localtime_r="yes" > - else > - localtime_r="no" > - fi > -fi > - > ########################################## > # pkg-config probe > > @@ -7088,9 +7057,6 @@ if [ "$bsd" = "yes" ] ; then > echo "CONFIG_BSD=y" >> $config_host_mak > fi > > -if test "$localtime_r" = "yes" ; then > - echo "CONFIG_LOCALTIME_R=y" >> $config_host_mak > -fi > if test "$qom_cast_debug" = "yes" ; then > echo "CONFIG_QOM_CAST_DEBUG=y" >> $config_host_mak > fi > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h > index d8978e28c0..3ac8a53bac 100644 > --- a/include/sysemu/os-win32.h > +++ b/include/sysemu/os-win32.h > @@ -48,12 +48,12 @@ > #define siglongjmp(env, val) longjmp(env, val) > > /* Missing POSIX functions. Don't use MinGW-w64 macros. */ > -#ifndef CONFIG_LOCALTIME_R > +#ifndef _POSIX_THREAD_SAFE_FUNCTIONS > #undef gmtime_r > struct tm *gmtime_r(const time_t *timep, struct tm *result); > #undef localtime_r > struct tm *localtime_r(const time_t *timep, struct tm *result); > -#endif /* CONFIG_LOCALTIME_R */ > +#endif > > static inline void os_setup_signal_handling(void) {} > static inline void os_daemonize(void) {} > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > index c654dafd93..f2fa9a3549 100644 > --- a/util/oslib-win32.c > +++ b/util/oslib-win32.c > @@ -106,7 +106,7 @@ void qemu_anon_ram_free(void *ptr, size_t size) > } > } > > -#ifndef CONFIG_LOCALTIME_R > +#ifndef _POSIX_THREAD_SAFE_FUNCTIONS > /* FIXME: add proper locking */ > struct tm *gmtime_r(const time_t *timep, struct tm *result) > { > -- > 2.28.0.windows.1 > > Regards, Daniel
On Tue, Sep 15, 2020 at 9:00 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Tue, Sep 15, 2020 at 08:12:56PM +0800, Yonggang Luo wrote: > > First, this reduce the size of configure, configure are tending to removal in future, > > and this didn't introduce any new feature or remove any exist feature. > > Second, the current localtime_r detection are conflict with ncursesw detection in > > mingw, when ncursesw detected, it will provide the following compile flags > > pkg-config --cflags ncursesw > > -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC:/CI-Tools/msys64/mingw64/include/ncursesw > > And the compile flag _POSIX_C_SOURCE will always cause _POSIX_THREAD_SAFE_FUNCTIONS to > > be defined, in new version of mingw, that's will cause localtime_r to be defined. > > But the configure script didn't provide _POSIX_C_SOURCE macro, and that's will result > > localtime_r not detected because localtime_r are defined in forceinline manner. > > ncursesw is just one of the three curses impls we can select for > building against, so it doesn't feel right to make an assumption > that _POSIX_C_SOURCE is always defined. That's what I am trying to do, not depends on if _POSIX_C_SOURCE are defined. After this patch, whenever ncursesw or other curses lib trying define or not define _POSIX_C_SOURCE, the source will building properly Because now, we don't make any assumption about _POSIX_C_SOURCE, but before this patch, The configure always assume msys2/mingw `not define _POSIX_C_SOURCE ` at all. Now this restriction are removed, only depends on mingw related _POSIX_THREAD_SAFE_FUNCTIONS > > > > > And finally cause conflict between QEMU defined localtime_r > > struct tm *localtime_r(const time_t *timep, struct tm *result); > > with mingw defined localtime_r > > > > ``` > > #if defined(_POSIX_C_SOURCE) && !defined(_POSIX_THREAD_SAFE_FUNCTIONS) > > #define _POSIX_THREAD_SAFE_FUNCTIONS 200112L > > #endif > > > > #ifdef _POSIX_THREAD_SAFE_FUNCTIONS > > __forceinline struct tm *__CRTDECL localtime_r(const time_t *_Time, struct tm *_Tm) { > > return localtime_s(_Tm, _Time) ? NULL : _Tm; > > } > > __forceinline struct tm *__CRTDECL gmtime_r(const time_t *_Time, struct tm *_Tm) { > > return gmtime_s(_Tm, _Time) ? NULL : _Tm; > > } > > __forceinline char *__CRTDECL ctime_r(const time_t *_Time, char *_Str) { > > return ctime_s(_Str, 0x7fffffff, _Time) ? NULL : _Str; > > } > > __forceinline char *__CRTDECL asctime_r(const struct tm *_Tm, char * _Str) { > > return asctime_s(_Str, 0x7fffffff, _Tm) ? NULL : _Str; > > } > > #endif > > ``` > > > > So I suggest remove this configure script, and restrict msys2/mingw version to easy to maintain. > > And use _POSIX_THREAD_SAFE_FUNCTIONS to guard the localtime_r and counterpart functions > > > > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com> > > --- > > configure | 34 ---------------------------------- > > include/sysemu/os-win32.h | 4 ++-- > > util/oslib-win32.c | 2 +- > > 3 files changed, 3 insertions(+), 37 deletions(-) > > > > diff --git a/configure b/configure > > index dc4b7a2e55..bac48b5b49 100755 > > --- a/configure > > +++ b/configure > > @@ -2496,37 +2496,6 @@ if test "$vhost_net" = ""; then > > test "$vhost_kernel" = "yes" && vhost_net=yes > > fi > > > > -########################################## > > -# MinGW / Mingw-w64 localtime_r/gmtime_r check > > - > > -if test "$mingw32" = "yes"; then > > - # Some versions of MinGW / Mingw-w64 lack localtime_r > > - # and gmtime_r entirely. > > - # > > - # Some versions of Mingw-w64 define a macro for > > - # localtime_r/gmtime_r. > > - # > > - # Some versions of Mingw-w64 will define functions > > - # for localtime_r/gmtime_r, but only if you have > > - # _POSIX_THREAD_SAFE_FUNCTIONS defined. For fun > > - # though, unistd.h and pthread.h both define > > - # that for you. > > - # > > - # So this #undef localtime_r and #include <unistd.h> > > - # are not in fact redundant. > > -cat > $TMPC << EOF > > -#include <unistd.h> > > -#include <time.h> > > -#undef localtime_r > > -int main(void) { localtime_r(NULL, NULL); return 0; } > > -EOF > > - if compile_prog "" "" ; then > > - localtime_r="yes" > > - else > > - localtime_r="no" > > - fi > > -fi > > - > > ########################################## > > # pkg-config probe > > > > @@ -7088,9 +7057,6 @@ if [ "$bsd" = "yes" ] ; then > > echo "CONFIG_BSD=y" >> $config_host_mak > > fi > > > > -if test "$localtime_r" = "yes" ; then > > - echo "CONFIG_LOCALTIME_R=y" >> $config_host_mak > > -fi > > if test "$qom_cast_debug" = "yes" ; then > > echo "CONFIG_QOM_CAST_DEBUG=y" >> $config_host_mak > > fi > > diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h > > index d8978e28c0..3ac8a53bac 100644 > > --- a/include/sysemu/os-win32.h > > +++ b/include/sysemu/os-win32.h > > @@ -48,12 +48,12 @@ > > #define siglongjmp(env, val) longjmp(env, val) > > > > /* Missing POSIX functions. Don't use MinGW-w64 macros. */ > > -#ifndef CONFIG_LOCALTIME_R > > +#ifndef _POSIX_THREAD_SAFE_FUNCTIONS > > #undef gmtime_r > > struct tm *gmtime_r(const time_t *timep, struct tm *result); > > #undef localtime_r > > struct tm *localtime_r(const time_t *timep, struct tm *result); > > -#endif /* CONFIG_LOCALTIME_R */ > > +#endif > > > > static inline void os_setup_signal_handling(void) {} > > static inline void os_daemonize(void) {} > > diff --git a/util/oslib-win32.c b/util/oslib-win32.c > > index c654dafd93..f2fa9a3549 100644 > > --- a/util/oslib-win32.c > > +++ b/util/oslib-win32.c > > @@ -106,7 +106,7 @@ void qemu_anon_ram_free(void *ptr, size_t size) > > } > > } > > > > -#ifndef CONFIG_LOCALTIME_R > > +#ifndef _POSIX_THREAD_SAFE_FUNCTIONS > > /* FIXME: add proper locking */ > > struct tm *gmtime_r(const time_t *timep, struct tm *result) > > { > > -- > > 2.28.0.windows.1 > > > > > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
diff --git a/configure b/configure index dc4b7a2e55..bac48b5b49 100755 --- a/configure +++ b/configure @@ -2496,37 +2496,6 @@ if test "$vhost_net" = ""; then test "$vhost_kernel" = "yes" && vhost_net=yes fi -########################################## -# MinGW / Mingw-w64 localtime_r/gmtime_r check - -if test "$mingw32" = "yes"; then - # Some versions of MinGW / Mingw-w64 lack localtime_r - # and gmtime_r entirely. - # - # Some versions of Mingw-w64 define a macro for - # localtime_r/gmtime_r. - # - # Some versions of Mingw-w64 will define functions - # for localtime_r/gmtime_r, but only if you have - # _POSIX_THREAD_SAFE_FUNCTIONS defined. For fun - # though, unistd.h and pthread.h both define - # that for you. - # - # So this #undef localtime_r and #include <unistd.h> - # are not in fact redundant. -cat > $TMPC << EOF -#include <unistd.h> -#include <time.h> -#undef localtime_r -int main(void) { localtime_r(NULL, NULL); return 0; } -EOF - if compile_prog "" "" ; then - localtime_r="yes" - else - localtime_r="no" - fi -fi - ########################################## # pkg-config probe @@ -7088,9 +7057,6 @@ if [ "$bsd" = "yes" ] ; then echo "CONFIG_BSD=y" >> $config_host_mak fi -if test "$localtime_r" = "yes" ; then - echo "CONFIG_LOCALTIME_R=y" >> $config_host_mak -fi if test "$qom_cast_debug" = "yes" ; then echo "CONFIG_QOM_CAST_DEBUG=y" >> $config_host_mak fi diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h index d8978e28c0..3ac8a53bac 100644 --- a/include/sysemu/os-win32.h +++ b/include/sysemu/os-win32.h @@ -48,12 +48,12 @@ #define siglongjmp(env, val) longjmp(env, val) /* Missing POSIX functions. Don't use MinGW-w64 macros. */ -#ifndef CONFIG_LOCALTIME_R +#ifndef _POSIX_THREAD_SAFE_FUNCTIONS #undef gmtime_r struct tm *gmtime_r(const time_t *timep, struct tm *result); #undef localtime_r struct tm *localtime_r(const time_t *timep, struct tm *result); -#endif /* CONFIG_LOCALTIME_R */ +#endif static inline void os_setup_signal_handling(void) {} static inline void os_daemonize(void) {} diff --git a/util/oslib-win32.c b/util/oslib-win32.c index c654dafd93..f2fa9a3549 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -106,7 +106,7 @@ void qemu_anon_ram_free(void *ptr, size_t size) } } -#ifndef CONFIG_LOCALTIME_R +#ifndef _POSIX_THREAD_SAFE_FUNCTIONS /* FIXME: add proper locking */ struct tm *gmtime_r(const time_t *timep, struct tm *result) {
First, this reduce the size of configure, configure are tending to removal in future, and this didn't introduce any new feature or remove any exist feature. Second, the current localtime_r detection are conflict with ncursesw detection in mingw, when ncursesw detected, it will provide the following compile flags pkg-config --cflags ncursesw -D_XOPEN_SOURCE=600 -D_POSIX_C_SOURCE=199506L -IC:/CI-Tools/msys64/mingw64/include/ncursesw And the compile flag _POSIX_C_SOURCE will always cause _POSIX_THREAD_SAFE_FUNCTIONS to be defined, in new version of mingw, that's will cause localtime_r to be defined. But the configure script didn't provide _POSIX_C_SOURCE macro, and that's will result localtime_r not detected because localtime_r are defined in forceinline manner. And finally cause conflict between QEMU defined localtime_r struct tm *localtime_r(const time_t *timep, struct tm *result); with mingw defined localtime_r ``` #if defined(_POSIX_C_SOURCE) && !defined(_POSIX_THREAD_SAFE_FUNCTIONS) #define _POSIX_THREAD_SAFE_FUNCTIONS 200112L #endif #ifdef _POSIX_THREAD_SAFE_FUNCTIONS __forceinline struct tm *__CRTDECL localtime_r(const time_t *_Time, struct tm *_Tm) { return localtime_s(_Tm, _Time) ? NULL : _Tm; } __forceinline struct tm *__CRTDECL gmtime_r(const time_t *_Time, struct tm *_Tm) { return gmtime_s(_Tm, _Time) ? NULL : _Tm; } __forceinline char *__CRTDECL ctime_r(const time_t *_Time, char *_Str) { return ctime_s(_Str, 0x7fffffff, _Time) ? NULL : _Str; } __forceinline char *__CRTDECL asctime_r(const struct tm *_Tm, char * _Str) { return asctime_s(_Str, 0x7fffffff, _Tm) ? NULL : _Str; } #endif ``` So I suggest remove this configure script, and restrict msys2/mingw version to easy to maintain. And use _POSIX_THREAD_SAFE_FUNCTIONS to guard the localtime_r and counterpart functions Signed-off-by: Yonggang Luo <luoyonggang@gmail.com> --- configure | 34 ---------------------------------- include/sysemu/os-win32.h | 4 ++-- util/oslib-win32.c | 2 +- 3 files changed, 3 insertions(+), 37 deletions(-)