Patch 8.1.2344

[Bram Moolenaar]

Patch 8.1.2344
Problem: Cygwin: warning for using strptime().
Solution: Move defining _XOPEN_SOURCE and __USE_XOPEN to vim.h. (Ken Takata,
closes #5265) Use 700 for _XOPEN_SOURCE for mkdtemp().
Files: src/os_unix.h, src/vim.h


*** ../vim-8.1.2343/src/os_unix.h 2019-11-21 15:36:00.245478796 +0100
--- src/os_unix.h 2019-11-26 13:11:43.968908469 +0100
***************
*** 129,141 ****

// on some systems time.h should not be included together with sys/time.h
#if !defined(HAVE_SYS_TIME_H) || defined(TIME_WITH_SYS_TIME)
- // Needed for strptime()
- # ifndef _XOPEN_SOURCE
- # define _XOPEN_SOURCE
- # endif
- # ifndef __USE_XOPEN
- # define __USE_XOPEN
- # endif
# include <time.h>
#endif
#ifdef HAVE_SYS_TIME_H
--- 129,134 ----
*** ../vim-8.1.2343/src/vim.h 2019-11-13 22:35:15.755521778 +0100
--- src/vim.h 2019-11-26 13:27:12.641051557 +0100
***************
*** 36,43 ****
Error: configure did not run properly. Check auto/config.log.
# endif

// for INT_MAX, LONG_MAX et al.
! #include <limits.h>

/*
* Cygwin may have fchdir() in a newer release, but in most versions it
--- 36,56 ----
Error: configure did not run properly. Check auto/config.log.
# endif

+ # ifdef UNIX
+ // Needed for strptime(). Needs to be done early, since header files can
+ // include other header files and end up including time.h, where these symbols
+ // matter for Vim.
+ // 700 is needed for mkdtemp().
+ # ifndef _XOPEN_SOURCE
+ # define _XOPEN_SOURCE 700
+ # endif
+ # ifndef __USE_XOPEN
+ # define __USE_XOPEN
+ # endif
+ # endif
+
// for INT_MAX, LONG_MAX et al.
! # include <limits.h>

/*
* Cygwin may have fchdir() in a newer release, but in most versions it
*** ../vim-8.1.2343/src/version.c 2019-11-26 12:23:26.679820852 +0100
--- src/version.c 2019-11-26 13:14:39.153266915 +0100
***************
*** 739,740 ****
--- 739,742 ----
{ /* Add new patch number below this line */
+ /**/
+ 2344,
/**/

--
hundred-and-one symptoms of being an internet addict:
124. You begin conversations with, "Who is your internet service provider?"

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Christian Brabandt]

On Di, 26 Nov 2019, Bram Moolenaar wrote:

> Patch 8.1.2344
> Problem: Cygwin: warning for using strptime().
> Solution: Move defining _XOPEN_SOURCE and __USE_XOPEN to vim.h. (Ken Takata,
> closes #5265) Use 700 for _XOPEN_SOURCE for mkdtemp().
> Files: src/os_unix.h, src/vim.h

This still breaks on BSD like systems.

How about the following patch on top of it?

diff --git a/src/vim.h b/src/vim.h
index 9b49ba1a9..c0053ce14 100644
--- a/src/vim.h
+++ b/src/vim.h
@@ -36,8 +36,8 @@

Error: configure did not run properly. Check auto/config.log.
# endif

-# if defined(UNIX) && !defined(MACOS_X)
-// Needed for strptime(). Needs to be done early, since header files can
+# if defined(UNIX) && !defined(MACOS_X) && !defined(__unix__)
+// Needed for strptime() on Cygwin. Needs to be done early, since header files can

// include other header files and end up including time.h, where these symbols

// matter for Vim.

// 700 is needed for mkdtemp().

Best,
Christian
--
Allen ist das Denken erlaubt. Vielen bleibt es erspart.

[Ken Takata]

Hi,

__unix__ is also defined on Cygwin. So, the warning will occur again.

The original issue looks Cygwin specific, so using __CYGWIN__ might be better:

    #ifdef __CYGWIN__

    // Needed for strptime() on Cygwin.  Needs to be done early, since header files can

    ...

Then os_unix.h needs to be reverted before 8.1.2344?

Regards,

Ken Takata

[Bram Moolenaar]

Christian wrote:

> On Di, 26 Nov 2019, Bram Moolenaar wrote:
>
> > Patch 8.1.2344
> > Problem: Cygwin: warning for using strptime().
> > Solution: Move defining _XOPEN_SOURCE and __USE_XOPEN to vim.h. (Ken Takata,
> > closes #5265) Use 700 for _XOPEN_SOURCE for mkdtemp().
> > Files: src/os_unix.h, src/vim.h
>
> This still breaks on BSD like systems.
>
> How about the following patch on top of it?
>
> diff --git a/src/vim.h b/src/vim.h
> index 9b49ba1a9..c0053ce14 100644
> --- a/src/vim.h
> +++ b/src/vim.h
> @@ -36,8 +36,8 @@
> Error: configure did not run properly. Check auto/config.log.
> # endif
>
> -# if defined(UNIX) && !defined(MACOS_X)
> -// Needed for strptime(). Needs to be done early, since header files can
> +# if defined(UNIX) && !defined(MACOS_X) && !defined(__unix__)

Why use __unix__ ? It's not used anywhere in Vim. There is an existing
check for BSD.

> +// Needed for strptime() on Cygwin. Needs to be done early, since header files can

It's also needed for Linux.

> // include other header files and end up including time.h, where these symbols
> // matter for Vim.
> // 700 is needed for mkdtemp().

--
Don't drink and drive. You might hit a bump and spill your beer.

[Christian Brabandt]

On Mi, 27 Nov 2019, Bram Moolenaar wrote:

>
> Christian wrote:
>
> > On Di, 26 Nov 2019, Bram Moolenaar wrote:
> >
> > > Patch 8.1.2344
> > > Problem: Cygwin: warning for using strptime().
> > > Solution: Move defining _XOPEN_SOURCE and __USE_XOPEN to vim.h. (Ken Takata,
> > > closes #5265) Use 700 for _XOPEN_SOURCE for mkdtemp().
> > > Files: src/os_unix.h, src/vim.h
> >
> > This still breaks on BSD like systems.
> >
> > How about the following patch on top of it?
> >
> > diff --git a/src/vim.h b/src/vim.h
> > index 9b49ba1a9..c0053ce14 100644
> > --- a/src/vim.h
> > +++ b/src/vim.h
> > @@ -36,8 +36,8 @@
> > Error: configure did not run properly. Check auto/config.log.
> > # endif
> >
> > -# if defined(UNIX) && !defined(MACOS_X)
> > -// Needed for strptime(). Needs to be done early, since header files can
> > +# if defined(UNIX) && !defined(MACOS_X) && !defined(__unix__)
>
> Why use __unix__ ? It's not used anywhere in Vim. There is an existing
> check for BSD.

__unix__ seems to be specific to well, unix like systems. I first tried,
defined(BSD), this did not work and is not defined (perhaps only
later?). Then I tried __FreeBSD__ that worked, but I assume it is too
specific. So I hoped __unix__ would do the trick.

Best,
Christian
--
Die eigene Erfahrung hat den Vorteil völliger Gewißheit.
-- Arthur Schopenhauer

[Bram Moolenaar]

FreeBSD and Cygwin are also a kind of Unix.

Defining _XOPEN_SOURCE in os_unix.h was probably a mistake. It should
be done before including anything, as the header files where it is
relevant can easily be included indirectly.

What was the error on FreeBSD? I thought it also suppored Posix.

I think being specific is OK, thus checking for __FreeBSD__ would be OK.
Otherwise we would need a configure check of some kind.

--
In a world without fences, who needs Gates and Windows?

[Christian Brabandt]

On Mi, 27 Nov 2019, Bram Moolenaar wrote:

> What was the error on FreeBSD? I thought it also suppored Posix.

https://cirrus-ci.com/task/5566942320001024

In file included from os_unix.c:87:
/usr/include/sys/consio.h:262:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short font_size;
^
/usr/include/sys/consio.h:263:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short mv_row, mv_col;
^
/usr/include/sys/consio.h:264:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short mv_rsz, mv_csz;
^
/usr/include/sys/consio.h:265:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short mv_hsz;
^
/usr/include/sys/consio.h:269:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char mv_ovscan;
^
/usr/include/sys/consio.h:270:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char mk_keylock;
^
/usr/include/sys/consio.h:319:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char ti_name[TI_NAME_LEN];
^
/usr/include/sys/consio.h:320:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char ti_desc[TI_DESC_LEN];
^
In file included from os_unix.c:88:
/usr/include/sys/fbio.h:196:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char *red; /* red color map elements */
^
/usr/include/sys/fbio.h:197:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char *green; /* green color map elements */
^
/usr/include/sys/fbio.h:198:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char *blue; /* blue color map elements */
^
/usr/include/sys/fbio.h:282:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short accessible_width; /* accessible bytes in scanline */
^
/usr/include/sys/fbio.h:283:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short accessible_height; /* number of accessible scanlines */
^
/usr/include/sys/fbio.h:284:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short line_bytes; /* number of bytes/scanline */
^
/usr/include/sys/fbio.h:285:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short hdb_capable; /* can this thing hardware db? */
^
/usr/include/sys/fbio.h:286:2: error: unknown type name 'u_short'; did you mean 'short'?
u_short vmsize; /* video memory size */
^
/usr/include/sys/fbio.h:287:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char boardrev; /* board revision # */
^
/usr/include/sys/fbio.h:288:2: error: unknown type name 'u_char'; did you mean 'char'?
u_char pad0;
^
/usr/include/sys/fbio.h:289:2: error: unknown type name 'u_long'; did you mean 'long'?
u_long pad1;
^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
gmake[2]: *** [Makefile:3338: objects/os_unix.o] Error 1
gmake[2]: Leaving directory '/tmp/cirrus-ci-build/src'
gmake[1]: *** [Makefile:2030: reconfig] Error 2
gmake[1]: Leaving directory '/tmp/cirrus-ci-build/src'
gmake: *** [Makefile:29: first] Error 2
Exit status: 2

>
> I think being specific is OK, thus checking for __FreeBSD__ would be OK.
> Otherwise we would need a configure check of some kind.
>

Thanks,
Christian
--
Letzte Worte eines Holz-Anstreichers:
"Xyladecor - für Innen- und Außenanstriche."

[Ken Takata]

Hi,

If we use __FreeBSD__, maybe we also need to add checks for other BSDs.

It seems that "BSD" is defined in sys/param.h, so how about this patch?

--- a/src/vim.h
+++ b/src/vim.h
@@ -36,7 +36,10 @@

     Error: configure did not run properly.  Check auto/config.log.
 # endif
 
-# if defined(UNIX) && !defined(MACOS_X)

+# ifdef HAVE_SYS_PARAM_H
+#  include <sys/param.h>
+# endif
+# if defined(UNIX) && !defined(MACOS_X) && !defined(BSD)

 // Needed for strptime().  Needs to be done early, since header files can

 // include other header files and end up including time.h, where these symbols
 // matter for Vim.

Regards,

Ken Takata

[Ken Takata]

Hi,

Sorry, this patch doesn't work as expected. This causes warnings on Cygwin again.

`&& !defined(__FreeBSD__)` might be needed. (And maybe similar checks for other BSDs?)

Regards,

Ken Takata

[Ken Takata]

Hi,

 How about this?

diff --git a/src/vim.h b/src/vim.h
--- a/src/vim.h
+++ b/src/vim.h
@@ -36,7 +36,7 @@

     Error: configure did not run properly.  Check auto/config.log.
 # endif
 
-# if defined(UNIX) && !defined(MACOS_X)

+# if defined(__gnu_linux__) || defined(__CYGWIN__)

 // Needed for strptime().  Needs to be done early, since header files can
 // include other header files and end up including time.h, where these symbols
 // matter for Vim.

Defining _XOPEN_SOURCE only on Linux and Cygwin might be enough?

Regards,

Ken Takata

[Christian Brabandt]

On Mi, 27 Nov 2019, Ken Takata wrote:

>  How about this?
>
> diff --git a/src/vim.h b/src/vim.h
> --- a/src/vim.h
> +++ b/src/vim.h
> @@ -36,7 +36,7 @@
>      Error: configure did not run properly.  Check auto/config.log.
>  # endif
>  
> -# if defined(UNIX) && !defined(MACOS_X)
> +# if defined(__gnu_linux__) || defined(__CYGWIN__)
>  // Needed for strptime().  Needs to be done early, since header files can
>  // include other header files and end up including time.h, where these symbols
>  // matter for Vim.
>
> Defining _XOPEN_SOURCE only on Linux and Cygwin might be enough?

Thanks, I am still confused whether _XOPEN_SOURCE needs to be defined
early only for Cygwin or for Cygwin and Linux. Your initial version (PR
5265) mentioned only Cygwin, so perhaps just #ifdef __CYGWIN__ would be
okay as well?

Best,
Christian
--
Diplomatie gleicht einem Boxkampf mit Glacéhandschuhen, bei dem der
Gong durch das Klingen der Sektgläser ersetzt wurde.
-- Georges Pompidou

[Bram Moolenaar]

Ken Takata wrote:

> >>> > What was the error on FreeBSD? I thought it also suppored Posix.
> >>>
> >>> https://cirrus-ci.com/task/5566942320001024
> >>>
> >>> In file included from os_unix.c:87:
> >>> /usr/include/sys/consio.h:262:2: error: unknown type name 'u_short'; did
> >>> you mean 'short'?
> >>> u_short font_size;
> >>> ^

[...]

That makes sense, since this mechanism is for all kinds of GNU-Linux,
but not for BSD-like systems. MacOS is also based on BSD.

--
hundred-and-one symptoms of being an internet addict:

132. You come back and check this list every half-hour.