RFC: Fl_Terminal::putchar() in public API

[Albrecht Schlosser]

Hi all,

TL;DR: I believe we should rename Fl_Terminal::putchar() to something that doesn't conflict with the global function name putchar() which *may* be defined as a macro.


I'm asking for your comments, see rationale below:

As GitHub issue #944 "Support for NetBSD" [1] shows, putchar() *can* be defined as a macro and Michael Bäuerle showed that this *is* the case for some NetBSD versions (maybe not all?).

Although you can use fully qualified Fl_Terminal::putchar(...) to distinguish Fl_Terminal's method from the global putchar() function this does not work if putchar is defined as a macro.

More info:

- According to the POSIX man page [2] for putchar(int c) "The function call putchar(c) shall be equivalent to putc(c,stdout)."
- According to the POSIX man page [3] for putc(int c, FILE *stream) "The putc() function shall be equivalent to fputc(), except that if it is implemented as a macro ..."

Honestly, I don't read this as "putchar() can be implemented as a macro" (emphasis on "The function call putchar(c) ..." in the docs) but as it stands, on the tested system putchar *is* defined as a macro.

Although Michael provides a patch for Fl_Terminal.cxx (short: #undef putchar) this is IMHO not a working solution because user code that calls Fl_Terminal::putchar() would also be affected even if fully qualified !

As a proof of concept I added one line to test/handle_keys.cxx:

win->tty->printf("Please press any key ...\n");

win->tty->Fl_Terminal::putchar('a', 0, 2); // this line added (line 337)

... which works as expected (note: fully qualified putchar call)

... until I also add the following macro after all #include's:

#define putchar(c) putc(c, stdout)

... as "suggested" by the POSIX docs cited above.


Error message in the user's program would be something like:

/git/fltk/master/test/handle_keys.cxx:337:43: error: macro "putchar" passed 3 arguments, but takes just 1
  337 |   win->tty->Fl_Terminal::putchar('a', 0, 2);
      |                                           ^
/path/to/fltk/test/handle_keys.cxx:29: note: macro "putchar" defined here
   29 | #define putchar(c) putc(c, stdout)
      |
/path/to/fltk/test/handle_keys.cxx: In function ‘int main(int, char**)’:
/path/to/fltk/test/handle_keys.cxx:337:26: error: statement cannot resolve address of overloaded function
  337 |   win->tty->Fl_Terminal::putchar('a', 0, 2);
      |   ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
ninja: build stopped: subcommand failed.

Conclusion:

Since we can't prevent that a system defines putchar as a macro we should IMHO rename the function rather than trying to work around it by patching the FLTK source file. And of course we can't '#undef putchar' anywhere in a public header file. Telling users that run into this issue to '#undef putchar' wouldn't be a good suggestion as well.

Another point is, BTW, that Fl_Terminal::putchar() has two overloaded methods, where one uses a 'char' but the other one uses a string ('char *') which might be confusing. Another name might clarify this.

My suggestion: Fl_Terminal::put_text() for both overloads. This avoids name clashes and 'text' is neutral and can take a 'char' or a 'char *'.

Fortunately it's still time to do this before we release 1.4.0 !

Comments and proposals appreciated.


Links:
[1] https://github.com/fltk/fltk/issues/944
[2] https://pubs.opengroup.org/onlinepubs/9699919799/functions/putchar.html
[3] https://pubs.opengroup.org/onlinepubs/9699919799/functions/putc.html

[Greg Ercolano]

On 3/25/24 12:21, 'Albrecht Schlosser' via fltk.coredev wrote:

Hi all,

TL;DR: I believe we should rename Fl_Terminal::putchar() to something that doesn't conflict with the global function name putchar() which *may* be defined as a macro.

Yuck.

We could rename it to putc() which, if POSIX says needs to be implemented
as a function (and not a macro), should be allowed. But, that still might cause
problems with the #define putchar() macro calling the wrong thing, for instance,
in a derived class where prefixes that would bypass the macro (like "->" and ".")
aren't needed.

There's a reason the method is implemented as putchar(), to emulate the
putchar() function within the widget. This should be legal to do, especially
as a class method.

And if calling it putc() would suffer similar interference with the putchar()
macro, then we couldn't use putc() either, which is just bad.

For this reason I'm not sure we should have to chase arguably broken platforms
like this. (If POSIX said "must be a macro" then I'd change my mind.)

I'd instead leave it to the application programmer to solve, like any other
messy platform specific issue, by adding "#undef putchar" before
"#include <FL/Fl_Terminal.H>" and similarly handle their use of putchar()
to non-fltk modules.

My feeling is take a stand and push the problem resolution back upstream
to its source, perhaps documenting the issue and how to workaround, assuming
the workaround is "workable".

[Albrecht Schlosser]

On 3/25/24 21:19 Greg Ercolano wrote:

On 3/25/24 12:21, 'Albrecht Schlosser' via fltk.coredev wrote:

Hi all,

TL;DR: I believe we should rename Fl_Terminal::putchar() to something that doesn't conflict with the global function name putchar() which *may* be defined as a macro.

We could rename it to putc() which, if POSIX says needs to be implemented

as a function (and not a macro), should be allowed. But, that still might cause
problems with the #define putchar() macro calling the wrong thing, for instance,
in a derived class where prefixes that would bypass the macro (like "->" and ".")
aren't needed.

Nope, putc() *can* be implemented as a macro, because the docs explicitly state "... except that if it is implemented as a macro it may ...". IMHO this says clearly that it is allowed to implement putc as a macro.

There's a reason the method is implemented as putchar(), to emulate the
putchar() function within the widget. This should be legal to do, especially
as a class method.

Unless this system function can be a macro. To clarify: I interpret the docs of putc clearly that it can be a macro but I didn't see clear evidence that putchar is allowed to be a macro. But I definitely don't know the details. My point is to avoid this issue by not naming the method like the system function.

What would be wrong with renaming the Fl_Terminal::putcha() method to avoid name clashes in the first place?

Regarding your argument to emulate putchar(): putchar() puts a single character (one byte!) and not a string (or a UTF-8 character) in an output stream. This is more like Fl_Terminal::append*() whereas Fl_Terminal::putchar() has additional arguments to write that char or string at an arbitrary place in the display.

I'd say this difference alone qualifies for a different method name (with or w/o taking the name clash into account).

And if calling it putc() would suffer similar interference with the putchar()
macro, then we couldn't use putc() either, which is just bad.

Definitely not putc because putc is AFAICT allowed to be a macro.

For this reason I'm not sure we should have to chase arguably broken platforms
like this. (If POSIX said "must be a macro" then I'd change my mind.)

I'm not sure if it's broken. It is only broken if `[int] putchar(int c)` *must not* be implemented as a macro. But this is not mentioned in the pages I linked above. I just don't know if it is allowed or not, that's one of the reasons why I asked for comments.

I'd instead leave it to the application programmer to solve, like any other
messy platform specific issue, by adding "#undef putchar" before
"#include <FL/Fl_Terminal.H>" and similarly handle their use of putchar()
to non-fltk modules.

They would also have to put "#undef putchar" after all include files that defines the putchar macro (I mean: after the end of the list), but they must not do it if another system include file depends on that macro. It's just not doable!

My feeling is take a stand and push the problem resolution back upstream
to its source, perhaps documenting the issue and how to workaround, assuming
the workaround is "workable".

This workaround is not "workable" in user code. What if the user wants to use Fl_Terminal::putchar(3args) and ::putchar(1arg) in the same source file and the system's putchar is defined as a macro? This doesn't work because you can't '#undef putchar'. #undef works in src/Fl_Terminal.cxx only because we don't use ::putchar in this file.

As long as we don't know which (maybe broken or not) systems define putchar() as a macro we can't say what is correct. You would get similar problems if you used Fl_Terminal::putc() and putc was legitimately defined as a macro.

[Greg Ercolano]

    TL;DR: If it's truly not possible to work around, then I suppose it has to change.

    Perhaps putchar_utf8(),

    And if POSIX explicitly allows it, well then I suppose we have to live
    with POSIX and all the warts it implies.

    But had 1.4.0 released with putchar() in place, I'd think there'd be
    possible workarounds.

On 3/25/24 14:00, 'Albrecht Schlosser' via fltk.coredev wrote:

What would be wrong with renaming the Fl_Terminal::putcha() method to avoid name clashes in the first place?

    Simple: API consistency.

    I implemented printf() and putchar() just to be consistent;
    for a terminal widget it's kinda important to have familiar stdio functions available.

    As the designer, I figured I should indicate why it should be the way
    it is, so folks can weigh their decisions.

    People seeing printf() implemented will be right to ask "why didn't
    you call the single character method putchar()?", and it just seems
    lame to have to say it's because of an obscure platform macro issue.

    But, if it's not possible to work around, then it sounds like there's
    no choice really, and it's not even up for question, other than what to call it.

    put_char() I guess?

[Michael Bäuerle]

'Albrecht Schlosser' via fltk.coredev wrote:
>

> [...]

> More info:
>
> - According to the POSIX man page [2] for putchar(int c) "The function

> call /putchar/(/c/) shall be equivalent to /putc/(/c/,/stdout/)."

> - According to the POSIX man page [3] for putc(int c, FILE *stream) "The

> /putc/() function shall be equivalent to /fputc/()
> <https://pubs.opengroup.org/onlinepubs/9699919799/functions/fputc.html>,

> except that if it is implemented as a macro ..."
>
> Honestly, I don't read this as "putchar() can be implemented as a macro"

> (emphasis on "The */function/* call /putchar/(/c/) ..." in the docs) but
> as it stands, on the tested system putchar **is** defined as a macro.

There are other Unix systems that seems to intentionally provide a
macro for putchar() and document it.

IBM documentation for AIX:
<https://www.ibm.com/docs/en/aix/7.2?topic=p-putc-putchar-fputc-putw-subroutine>
|
| The putchar macro is the same as the putc macro except that
| putchar writes to the standard output.

Oracle documentation for Solaris:
<https://docs.oracle.com/cd/E88353_01/html/E37843/putchar-3c.html>
|
| The call putchar(c) is equivalent to putc(c, stdout).
| The putchar() routine is implemented as a macro.

[melcher....@googlemail.com]

I m following this with a certain awe. Defining a macro has tons of side effects, and defining such simple word as `putchar` in a system header is horrible. Anyway, we can fight macros with macros... .

Not sure it that would compile, or work in this context, but if we define our own macro, we should be able to generate the same code, even if putchar is a macro. I would add `putc` as a method, so even if there is #define putchar(x) putc(x, stdout) somewhere, we would still compile correctly:

#define putchar(x) putc(x, stdout)

class Fl_Terminal {

  int putc(int c, FILE *); // 2nd arg is ignored

#ifdef putchar

  #define FL_TERMINAL_PUTCHAR putchar

  int FL_TERMINAL_PUTCHAR(int c);

  #undef FL_TERMINAL_PUTCHAR

#else

  int putchar(int c);

#endif

}

so calling myTerminal->putchar('a'); should do the right thing in either case, and Fl_Terminal::putchar() should exist, even if putchar is a macro.

[Albrecht Schlosser]

On 3/27/24 17:27 'melcher....@googlemail.com' via fltk.coredev wrote:

I m following this with a certain awe. Defining a macro has tons of side effects, and defining such simple word as `putchar` in a system header is horrible.

I agree with this statement. 100%.

Anyway, we can fight macros with macros... .

I don't think this would work, even if we wanted, see below.

Not sure it that would compile, or work in this context, but if we define our own macro, we should be able to generate the same code, even if putchar is a macro. I would add `putc` as a method, so even if there is #define putchar(x) putc(x, stdout) somewhere, we would still compile correctly:

#define putchar(x) putc(x, stdout)

class Fl_Terminal {

  int putc(int c, FILE *); // 2nd arg is ignored

#ifdef putchar

  #define FL_TERMINAL_PUTCHAR putchar

  int FL_TERMINAL_PUTCHAR(int c);

  #undef FL_TERMINAL_PUTCHAR

#else

  int putchar(int c);

#endif

}

so calling myTerminal->putchar('a'); should do the right thing in either case, and Fl_Terminal::putchar() should exist, even if putchar is a macro.

Fortunately [1] this doesn't work for several reasons:

1. The method in Fl_Terminal is putchar(...) with either three or four parameters whereas the well-known putchar(c) macro has only one parameter. In a quick test I believe that I saw that the preprocessor chokes already (wrong number of arguments).

2. Lines with macros are interpreted iteratively until no macro expansion takes place anymore. Thus, even if we defined "FL_TERMINAL_PUTCHAR" as "putchar", then putchar would be replaced with "putc()" again. And, again, the number of parameters doesn't match.

3. I don't think (i.e. I'm not sure) that this construct would be immune against defining 'putchar' either before or after the inclusion of Fl_Terminal.H if it would make it through the preprocessor at all. Maybe it works, maybe not, I'm not willing to "test" this though.

That said, I'm glad that this doesn't work (AFAICT). It would only add more to the confusion. Trying to make this work would IMHO be a waste of time, particularly because there's a very simple solution: rename the offending Fl_Terminal::putchar(...) method.


Footnote:

[1] Yes, I mean "fortunately". This idea would IMHO make the code less transparent, much harder to maintain ("why does this weird construct of macros exist in the first place), and supposedly more fragile. What would happen if putchar is defined as a macro with an entirely different meaning or set of parameters?

[Albrecht Schlosser]

On 3/27/24 11:02 Michael Bäuerle wrote:

There are other Unix systems that seems to intentionally provide a

macro for putchar() and document it.

IBM documentation for AIX:
<https://www.ibm.com/docs/en/aix/7.2?topic=p-putc-putchar-fputc-putw-subroutine>
|
| The putchar macro is the same as the putc macro except that
| putchar writes to the standard output.

Oracle documentation for Solaris:
<https://docs.oracle.com/cd/E88353_01/html/E37843/putchar-3c.html>
|
| The call putchar(c) is equivalent to putc(c, stdout).
| The putchar() routine is implemented as a macro.

Michael, thank you for posting these links.

At least the AIX docs make it clear that putchar() is a macro. Nothing else.

The Solaris docs say "Functions exist for the putc(), putc_unlocked(), putchar(), and putchar_unlocked() macros. To get the function form, the macro name must be undefined (for example, #undef putc)."

No matter if this is suitable, but #undef'ing the macro would have to be done by the user and not within an FLTK header file. Telling the user to '#undef putchar' to be able to use Fl_Terminal would obviously be a bad choice because it would interfere with a system function. Besides that we don't know if these two systems are only the tip of the iceberg.


Conclusion: I don't see another option than renaming Fl_Terminal::putchar() to something else. As Greg proposed, I would favor Fl_Terminal::put_char() - with an additional underscore. This would retain the meaning (intention) but make it different enough.

We should come to a decision soon. Are there any objections left ?

Note: I don't think that a vote is appropriate here, other than "voting" which name to choose. The renaming must be done, otherwise we'd lose the ability to use FLTK on at least two mainstream platforms. Both docs have been updated recently (Solaris: 2022, AIX: 2023), i.e. these platforms are obviously still alive.

[Bill Spitzak]

I think you could just call it put() because that can be overloaded with different argument types so you can print characters or strings, and maybe some other types of data.

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/d89390e4-6985-456a-8321-a656bf0dc74d%40aljus.de.

[Greg Ercolano]

put() or put_char() works for me -- shorter is better IMHO.

[Albrecht Schlosser]

On 4/2/24 05:31 Greg Ercolano wrote:
>
> put() or put_char() works for me -- shorter is better IMHO.
>

I like put() as well, that's IMHO a good choice. Let's go with put().

Greg, would you like to update the code, or would you like me to do it?

> On 4/1/24 14:51, Bill Spitzak wrote:
>> I think you could just call it put() because that can be overloaded
>> with different argument types so you can print characters or strings,
>> and maybe some other types of data.

Yup, in 1.5 we might want to add std::string as another argument type.

[melcher....@googlemail.com]

Just for completeness, the code below uses some macro foo to circumvent the putchar macro. 

#define putchar(a) putc(a, stdout)

#define empty()

#define delay(f) f empty()

// The function name will actually be `putchar`!

int delay(putchar) (int x) {

  return x;

}

//#undef putchar

int main(int argc, char **argv) {

  putchar('2');

}

[Greg Ercolano]

On 4/2/24 05:14, 'Albrecht Schlosser' via fltk.coredev wrote:

On 4/2/24 05:31 Greg Ercolano wrote:

put() or put_char() works for me -- shorter is better IMHO.

I like put() as well, that's IMHO a good choice. Let's go with put().
Greg, would you like to update the code, or would you like me to do it?

Looking into the code, there's an existing method "print_char()" with a few variations:

  void putchar(const char *text, int len, int drow, int dcol);
  void putchar(char c, int drow, int dcol);
  void print_char(const char *text, int len=-1);
  void print_char(char c);

The print_char() methods:

    advance the cursor
    handles control codes like \n, \t, \r, etc.
    handles ANSI esc sequences
    Must be a single character (ascii or utf8)

The putchar() methods:

    Doesn't move the cursor
    Don't handle ANSI escape sequences
    Don't handle control chars
    Must be a single character (ascii or utf8)

"char" seems important to keep in the name, so as not to be confused with
methods that handle multichar strings (like printf() and append()).

So I think put() shouldn't be used for that reason.

Since putchar() also doesn't move the cursor, perhaps a better name is plot_char(),
since it's more like plotting pixels than "printing".

Given all that, I'm thinking plot_char() or plotchar().

Suggs?

[Albrecht Schlosser]

On 4/2/24 17:36 Greg Ercolano wrote:

On 4/2/24 05:14, 'Albrecht Schlosser' via fltk.coredev wrote:

On 4/2/24 05:31 Greg Ercolano wrote:

put() or put_char() works for me -- shorter is better IMHO.

I like put() as well, that's IMHO a good choice. Let's go with put().
Greg, would you like to update the code, or would you like me to do it?

Looking into the code, there's an existing method "print_char()" with a few variations:

  void putchar(const char *text, int len, int drow, int dcol);
  void putchar(char c, int drow, int dcol);
  void print_char(const char *text, int len=-1);
  void print_char(char c);

The print_char() methods:

    advance the cursor
    handles control codes like \n, \t, \r, etc.
    handles ANSI esc sequences
    Must be a single character (ascii or utf8)

The putchar() methods:

    Doesn't move the cursor
    Don't handle ANSI escape sequences
    Don't handle control chars
    Must be a single character (ascii or utf8)

"char" seems important to keep in the name, so as not to be confused with
methods that handle multichar strings (like printf() and append()).

So I think put() shouldn't be used for that reason.

I agree.

Since putchar() also doesn't move the cursor, perhaps a better name is plot_char(),
since it's more like plotting pixels than "printing".

But a plotter would also move its pen ("cursor"), wouldn't it? ;-)

Given all that, I'm thinking plot_char() or plotchar().

Since we have print_char (with underscore) I would prefer plot_char() rather than plotchar() for consistency and general FLTK conventions.

However, for the reason above (see "plotter") and your reasoning I propose to return to put_char().

My personal preference (highest preference on top):

1. put_char()
2. plot_char()
3. put() - but see above why not
3. anything else - unless there are even better suggestions.

Other opinions?

[Greg Ercolano]

On 4/2/24 08:51, 'Albrecht Schlosser' via fltk.coredev wrote:

Since putchar() also doesn't move the cursor, perhaps a better name is plot_char(),
since it's more like plotting pixels than "printing".

But a plotter would also move its pen ("cursor"), wouldn't it? ;-)

    Heh, I was thinking more of plotting pixels.

    But if people actually remember what plotters (with pens) are, then put_char() it shall be.

    I won't be free to do anything until much later tonight, so if you all come up with something better between now and then, I'm all ears..

[Albrecht Schlosser]

On 4/2/24 18:08 Greg Ercolano wrote:
>
>     Heh, I was thinking more of plotting pixels.
>
>     But if people actually remember what plotters (with pens) are,
> then put_char() it shall be.

+1

[Bill Spitzak]

I would really expect "putchar" or any variation to actually act like that character was typed on the terminal, including handling ESC and CR, etc. And moving the cursor.

A "plotchar" would take an x,y position, imho

I guess overloading could still use the same name as the position makes the argument list different.

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/761f17f3-67de-42aa-b16a-a9d32ea6ccdb%40aljus.de.

[Greg Ercolano]

Holding for a reply from Albrecht, as he's the one that didn't like the 'plot' idea.

I do think plot is the better choice given the x,y args and the "does not affect the
cursor position" behavior. (Confusion about pen plotter behavior seems like a stretch)
I also give weight to Bill's opinion because he gifted us FLTK.

But I'm open to whatever you guys (or anyone else) can agree on, and will
implement that.

[Albrecht Schlosser]

No problem here, I'd be fine with plot_char(), if that's preferred by you and others.

--
You received this message because you are subscribed to the Google Groups "fltk.coredev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to fltkcoredev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/fltkcoredev/6b58a150-4a78-4618-8e95-dbdde2caadf1%40seriss.com.

[Greg Ercolano]

On 4/4/24 10:23, 'Albrecht Schlosser' via fltk.coredev wrote:

No problem here, I'd be fine with plot_char(), if that's preferred by you and others.

    Done; plot_char() in 31170c4.