diff mbox

curses: build with -std=gnu99

Message ID 1477568774-4817-1-git-send-email-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gerd Hoffmann Oct. 27, 2016, 11:46 a.m. UTC
---
 ui/Makefile.objs | 3 +++
 1 file changed, 3 insertions(+)

Comments

Peter Maydell Oct. 27, 2016, 3:04 p.m. UTC | #1
On 27 October 2016 at 12:46, Gerd Hoffmann <kraxel@redhat.com> wrote:
> ---
>  ui/Makefile.objs | 3 +++
>  1 file changed, 3 insertions(+)

Missing commit message and signed-off-by line...

> diff --git a/ui/Makefile.objs b/ui/Makefile.objs
> index dc936f1..62f4cf3 100644
> --- a/ui/Makefile.objs
> +++ b/ui/Makefile.objs
> @@ -17,6 +17,9 @@ common-obj-$(CONFIG_CURSES) += curses.o
>  common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
>  common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o
>
> +# needed to make gcc accept wide unicode chars without warning
> +curses.o-cflags := -std=gnu99
> +
>  ifeq ($(CONFIG_SDLABI),1.2)
>  sdl.mo-objs := sdl.o sdl_zoom.o
>  endif
> --
> 1.8.3.1

I'm not sure about this.

Do we really want gnu99 and not c99? Should we just enable
std=c99 for all source files, given that we already assume
C99 anyway?

It would also be helpful if the commit message quoted the
compiler warning that you get otherwise, so it's easier
to see why we're doing this.

thanks
-- PMM
Gerd Hoffmann Oct. 27, 2016, 3:55 p.m. UTC | #2
Hi,

> > +# needed to make gcc accept wide unicode chars without warning
> > +curses.o-cflags := -std=gnu99
> > +
> >  ifeq ($(CONFIG_SDLABI),1.2)
> >  sdl.mo-objs := sdl.o sdl_zoom.o
> >  endif
> > --
> > 1.8.3.1
> 
> I'm not sure about this.
> 
> Do we really want gnu99 and not c99? Should we just enable
> std=c99 for all source files, given that we already assume
> C99 anyway?

-std=c99 doesn't fly, throws errors on (gcc-specific) asm.
So we have to use -std=gnu99.

Tried to do that tree-wide by patching configure but got build errors
then (somewhere in iscsi code).

> It would also be helpful if the commit message quoted the
> compiler warning that you get otherwise, so it's easier
> to see why we're doing this.

With the wide char curses patches by samuel applied I get errors like
this one:

/home/kraxel/projects/qemu/ui/curses.c:627:18: error: universal
character names are only valid in C++ and C99 [-Werror]
             case L'\u23bd':
                  ^

I'm open to better ideas to handle this.

Failing that I can cut+paste this reply into the commit message to
clarify things.

cheers,
  Gerd
Samuel Thibault Oct. 27, 2016, 3:58 p.m. UTC | #3
Gerd Hoffmann, on Thu 27 Oct 2016 17:55:47 +0200, wrote:
> /home/kraxel/projects/qemu/ui/curses.c:627:18: error: universal
> character names are only valid in C++ and C99 [-Werror]
>              case L'\u23bd':

Another way could be to assume unicode encoding of wchar_t characters
(which looks very reasonable to me) and just write "case 0x23bd:".

Samuel
Peter Maydell Oct. 27, 2016, 4:14 p.m. UTC | #4
On 27 October 2016 at 16:58, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> Gerd Hoffmann, on Thu 27 Oct 2016 17:55:47 +0200, wrote:
>> /home/kraxel/projects/qemu/ui/curses.c:627:18: error: universal
>> character names are only valid in C++ and C99 [-Werror]
>>              case L'\u23bd':
>
> Another way could be to assume unicode encoding of wchar_t characters
> (which looks very reasonable to me) and just write "case 0x23bd:".

Does this still work if you're using curses on mingw32?

thanks
-- PMM
Samuel Thibault Oct. 27, 2016, 4:36 p.m. UTC | #5
Peter Maydell, on Thu 27 Oct 2016 17:14:52 +0100, wrote:
> On 27 October 2016 at 16:58, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > Gerd Hoffmann, on Thu 27 Oct 2016 17:55:47 +0200, wrote:
> >> /home/kraxel/projects/qemu/ui/curses.c:627:18: error: universal
> >> character names are only valid in C++ and C99 [-Werror]
> >>              case L'\u23bd':
> >
> > Another way could be to assume unicode encoding of wchar_t characters
> > (which looks very reasonable to me) and just write "case 0x23bd:".
> 
> Does this still work if you're using curses on mingw32?

Windows' wchar_t uses unicode encoding, yes (and its limitation to 16bit
doesn't pose problem to the values we care about).

Samuel
Peter Maydell Oct. 27, 2016, 4:52 p.m. UTC | #6
On 27 October 2016 at 17:36, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> Peter Maydell, on Thu 27 Oct 2016 17:14:52 +0100, wrote:
>> On 27 October 2016 at 16:58, Samuel Thibault <samuel.thibault@gnu.org> wrote:
>> > Gerd Hoffmann, on Thu 27 Oct 2016 17:55:47 +0200, wrote:
>> >> /home/kraxel/projects/qemu/ui/curses.c:627:18: error: universal
>> >> character names are only valid in C++ and C99 [-Werror]
>> >>              case L'\u23bd':
>> >
>> > Another way could be to assume unicode encoding of wchar_t characters
>> > (which looks very reasonable to me) and just write "case 0x23bd:".
>>
>> Does this still work if you're using curses on mingw32?
>
> Windows' wchar_t uses unicode encoding, yes (and its limitation to 16bit
> doesn't pose problem to the values we care about).

On the other hand apparently FreeBSD and Solaris have a wchar_t
whose encoding is locale-dependent:

http://www.thecodingforums.com/threads/wchar_t-is-useless.806149/#post-4398211

thanks
-- PMM
Samuel Thibault Oct. 27, 2016, 5:02 p.m. UTC | #7
Peter Maydell, on Thu 27 Oct 2016 17:52:17 +0100, wrote:
> On 27 October 2016 at 17:36, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> > Peter Maydell, on Thu 27 Oct 2016 17:14:52 +0100, wrote:
> >> On 27 October 2016 at 16:58, Samuel Thibault <samuel.thibault@gnu.org> wrote:
> >> > Gerd Hoffmann, on Thu 27 Oct 2016 17:55:47 +0200, wrote:
> >> >> /home/kraxel/projects/qemu/ui/curses.c:627:18: error: universal
> >> >> character names are only valid in C++ and C99 [-Werror]
> >> >>              case L'\u23bd':
> >> >
> >> > Another way could be to assume unicode encoding of wchar_t characters
> >> > (which looks very reasonable to me) and just write "case 0x23bd:".
> >>
> >> Does this still work if you're using curses on mingw32?
> >
> > Windows' wchar_t uses unicode encoding, yes (and its limitation to 16bit
> > doesn't pose problem to the values we care about).
> 
> On the other hand apparently FreeBSD and Solaris have a wchar_t
> whose encoding is locale-dependent:
> 
> http://www.thecodingforums.com/threads/wchar_t-is-useless.806149/#post-4398211

UURrgll... So we can't use L'\u23bd' on such systems, it would just not
work either, we have to use iconv to get these right...

Samuel
Markus Armbruster Oct. 28, 2016, 6:51 a.m. UTC | #8
Samuel Thibault <samuel.thibault@gnu.org> writes:

> Peter Maydell, on Thu 27 Oct 2016 17:52:17 +0100, wrote:
>> On 27 October 2016 at 17:36, Samuel Thibault <samuel.thibault@gnu.org> wrote:
>> > Peter Maydell, on Thu 27 Oct 2016 17:14:52 +0100, wrote:
>> >> On 27 October 2016 at 16:58, Samuel Thibault <samuel.thibault@gnu.org> wrote:
>> >> > Gerd Hoffmann, on Thu 27 Oct 2016 17:55:47 +0200, wrote:
>> >> >> /home/kraxel/projects/qemu/ui/curses.c:627:18: error: universal
>> >> >> character names are only valid in C++ and C99 [-Werror]
>> >> >>              case L'\u23bd':
>> >> >
>> >> > Another way could be to assume unicode encoding of wchar_t characters
>> >> > (which looks very reasonable to me) and just write "case 0x23bd:".
>> >>
>> >> Does this still work if you're using curses on mingw32?
>> >
>> > Windows' wchar_t uses unicode encoding, yes (and its limitation to 16bit
>> > doesn't pose problem to the values we care about).
>> 
>> On the other hand apparently FreeBSD and Solaris have a wchar_t
>> whose encoding is locale-dependent:
>> 
>> http://www.thecodingforums.com/threads/wchar_t-is-useless.806149/#post-4398211
>
> UURrgll... So we can't use L'\u23bd' on such systems, it would just not
> work either, we have to use iconv to get these right...

I guess we can if they actually bother to conform to C99.  6.4.3
Universal character names:

    The universal character name \Unnnnnnnn designates the character
    whose eight-digit short identifier (as specified by ISO/IEC 10646)
    is nnnnnnnn.  Similarly, the universal character name \unnnn
    designates the character whose four-digit short identifier is nnnn
    (and whose eight-digit short identifier is 0000nnnn).

Note that it doesn't say here that L'\u23bd' should be equal to 0x23bd.
Samuel Thibault Oct. 28, 2016, 7:40 a.m. UTC | #9
Markus Armbruster, on Fri 28 Oct 2016 08:51:20 +0200, wrote:
> Samuel Thibault <samuel.thibault@gnu.org> writes:
> 
> > Peter Maydell, on Thu 27 Oct 2016 17:52:17 +0100, wrote:
> >> http://www.thecodingforums.com/threads/wchar_t-is-useless.806149/#post-4398211
> >
> > UURrgll... So we can't use L'\u23bd' on such systems, it would just not
> > work either, we have to use iconv to get these right...
> 
> I guess we can if they actually bother to conform to C99.  6.4.3
> Universal character names:
> 
>     The universal character name \Unnnnnnnn designates the character
>     whose eight-digit short identifier (as specified by ISO/IEC 10646)
>     is nnnnnnnn.  Similarly, the universal character name \unnnn
>     designates the character whose four-digit short identifier is nnnn
>     (and whose eight-digit short identifier is 0000nnnn).
> 
> Note that it doesn't say here that L'\u23bd' should be equal to 0x23bd.

I know, but as discussed in the thread, that could only work if gcc
embeds something like calls to nl_langinfo and iconv to properly convert
from the unicode value to the proper wchar_t according to the locale.  I
wouldn't be surprised that they don't.

Samuel
Markus Armbruster Oct. 28, 2016, 8:37 a.m. UTC | #10
Samuel Thibault <samuel.thibault@gnu.org> writes:

> Markus Armbruster, on Fri 28 Oct 2016 08:51:20 +0200, wrote:
>> Samuel Thibault <samuel.thibault@gnu.org> writes:
>> 
>> > Peter Maydell, on Thu 27 Oct 2016 17:52:17 +0100, wrote:
>> >> http://www.thecodingforums.com/threads/wchar_t-is-useless.806149/#post-4398211
>> >
>> > UURrgll... So we can't use L'\u23bd' on such systems, it would just not
>> > work either, we have to use iconv to get these right...
>> 
>> I guess we can if they actually bother to conform to C99.  6.4.3
>> Universal character names:
>> 
>>     The universal character name \Unnnnnnnn designates the character
>>     whose eight-digit short identifier (as specified by ISO/IEC 10646)
>>     is nnnnnnnn.  Similarly, the universal character name \unnnn
>>     designates the character whose four-digit short identifier is nnnn
>>     (and whose eight-digit short identifier is 0000nnnn).
>> 
>> Note that it doesn't say here that L'\u23bd' should be equal to 0x23bd.
>
> I know, but as discussed in the thread, that could only work if gcc
> embeds something like calls to nl_langinfo and iconv to properly convert
> from the unicode value to the proper wchar_t according to the locale.  I
> wouldn't be surprised that they don't.

Before we uglify our code to work around (egregious!) defects in certain
compilers, we should find out whether and where exactly these defects
exist, and whether it's worth targeting the defective compilers.
Gerd Hoffmann Oct. 28, 2016, 9:45 a.m. UTC | #11
Hi,

> Before we uglify our code to work around (egregious!) defects in certain
> compilers, we should find out whether and where exactly these defects
> exist, and whether it's worth targeting the defective compilers.

Using iconv is on the table _anyway_, to properly support
"-display curses,charset=$something", so using that for the cp437 table
too actually simplifies things and avoids hard-coding tables using
\uxxxx as side effect.

cheers,
  Gerd
diff mbox

Patch

diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index dc936f1..62f4cf3 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -17,6 +17,9 @@  common-obj-$(CONFIG_CURSES) += curses.o
 common-obj-$(CONFIG_VNC) += $(vnc-obj-y)
 common-obj-$(CONFIG_GTK) += gtk.o x_keymap.o
 
+# needed to make gcc accept wide unicode chars without warning
+curses.o-cflags := -std=gnu99
+
 ifeq ($(CONFIG_SDLABI),1.2)
 sdl.mo-objs := sdl.o sdl_zoom.o
 endif