fltk-1.4 fluid build broken on 32-bit mingw

[imacarthur]

This has likely been broken (for me) for a bit, but I only checked the logs from my build script today, after trying to build on a new WIn10 laptop...

FWIW, the 64-bit build is fine, which possibly explains why I never noticed there was an issue: The build machine runs a different gcc variant for the 32-bit and 64-bit targets, for obscure historical reasons...

The 32-bit version reports itself as: gcc version 6.3.0 (MinGW.org GCC-6.3.0-1)

The 64-bit version is: gcc version 5.1.0 (tdm64-1)

Anyway, here's the error:

Compiling fluid.cxx...

In file included from ../FL/fl_utf8.h:32:0,

                 from ../FL/Fl.H:32,

                 from fluid.cxx:17:

fluid.cxx:1356:7: error: expected unqualified-id before ')' token

   int fileno() const {

       ^

fluid.cxx:1356:7: error: expected ')' before '->' token

   int fileno() const {

       ^

fluid.cxx: In function 'void shell_pipe_cb(FL_SOCKET, void*)':

fluid.cxx:1504:26: error: expected unqualified-id before '(' token

     Fl::remove_fd(s_proc.fileno());

                          ^

fluid.cxx:1504:26: error: expected primary-expression before ')' token

     Fl::remove_fd(s_proc.fileno());

                          ^

fluid.cxx: In function 'void do_shell_command(Fl_Return_Button*, void*)':

fluid.cxx:1540:21: error: expected unqualified-id before '(' token

   Fl::add_fd(s_proc.fileno(), shell_pipe_cb);

                     ^

fluid.cxx:1540:21: error: expected primary-expression before ')' token

   Fl::add_fd(s_proc.fileno(), shell_pipe_cb);

                     ^

make[1]: *** [fluid.o] Error 1

The crux is this code, added to fluid.cxx in 1.4:

  // returns fileno(FILE*):

  // (file must be open, i.e. _fpt must be non-null)

  // *FIXME* we should find a better solution for the 'fileno' issue

  int fileno() const {

#ifdef _MSC_VER

    return _fileno(_fpt); // suppress MSVC warning

#else

    return ::fileno(_fpt);

#endif

  } // non null if file is open

Which chokes on my 32-bit build because the "stdio.h" in that system defines:

#define _fileno(__F) ((__F)->_file)

#ifndef _NO_OLDNAMES

#define fileno(__F) ((__F)->_file)   <-- this is the problem in my build!

#endif

And that #define then chokes the fluid.cxx code, preventing it from compiling...

[Albrecht Schlosser]

On 9/27/21 1:14 PM imacarthur wrote:

This has likely been broken (for me) for a bit, but I only checked the logs from my build script today, after trying to build on a new WIn10 laptop...

FWIW, the 64-bit build is fine, which possibly explains why I never noticed there was an issue: The build machine runs a different gcc variant for the 32-bit and 64-bit targets, for obscure historical reasons...

...

The crux is this code, added to fluid.cxx in 1.4:

  // returns fileno(FILE*):

  // (file must be open, i.e. _fpt must be non-null)

  // *FIXME* we should find a better solution for the 'fileno' issue

  int fileno() const {

#ifdef _MSC_VER

    return _fileno(_fpt); // suppress MSVC warning

#else

    return ::fileno(_fpt);

#endif

  } // non null if file is open

Which chokes on my 32-bit build because the "stdio.h" in that system defines: [...]

Ooh, I wouldn't have expected that a (MinGW) system defines a macro for fileno. I tried to fix one of these silly MS warnings, but using 'fileno' as a method name seems to have been a bad idea.

I'm afraid I can't build in that environment, so I'd like to ask you to try a fix:

We can probably fix this by renaming our own 'fileno' method to 'filno' to avoid the name clash, see attached diff. Can you please try if this fixes the issue?

Just in case '::fileno()' chokes as well, you can try to remove the '::' since we shouldn't have a conflict after the rename to 'filno'.

[Greg Ercolano]

On 9/27/21 11:17 AM, Albrecht Schlosser wrote:

We can probably fix this by renaming our own 'fileno' method to 'filno' to avoid the name clash..

    When we commit, fluid_fileno() might be a better name, since it's fluid specific.
    Or perhaps we need a driver and a public fl_fileno()..?

[Albrecht Schlosser]

On 9/27/21 8:25 PM Greg Ercolano wrote:
>
> On 9/27/21 11:17 AM, Albrecht Schlosser wrote:
>
>> We can probably fix this by renaming our own 'fileno' method to
>> 'filno' to avoid the name clash..
>
>     When we commit, fluid_fileno() might be a better name, since it's
> fluid specific.
>

Greg, this is all about a method of a local class in exactly one fluid
source file, hence method names are kinda irrelevant (and not public at
all). The "FIXME" comment was intended as reminder for a better
solution, maybe ...

>     Or perhaps we need a driver and a public fl_fileno()..?
>

This "fix" was only to remove MS (VS) warnings and intended as a quick
shot to avoid having to add a public fl_fileno() function with a system
driver implementation. The latter seemed to be overkill because we
needed it only in one place (IIRC there was another place I fixed
locally as well, but that doesn't matter here).

[Ian MacArthur]

On 27 Sep 2021, at 19:17, Albrecht Schlosser wrote:
>
> On 9/27/21 1:14 PM imacarthur wrote:
>> This has likely been broken (for me) for a bit, but I only checked the logs from my build script today, after trying to build on a new WIn10 laptop...
>>
>> FWIW, the 64-bit build is fine, which possibly explains why I never noticed there was an issue: The build machine runs a different gcc variant for the 32-bit and 64-bit targets, for obscure historical reasons...
>> ...
>> The crux is this code, added to fluid.cxx in 1.4:
>>
>> // returns fileno(FILE*):
>> // (file must be open, i.e. _fpt must be non-null)
>> // *FIXME* we should find a better solution for the 'fileno' issue
>> int fileno() const {
>> #ifdef _MSC_VER
>> return _fileno(_fpt); // suppress MSVC warning
>> #else
>> return ::fileno(_fpt);
>> #endif
>> } // non null if file is open
>>
>> Which chokes on my 32-bit build because the "stdio.h" in that system defines: [...]
>
> Ooh, I wouldn't have expected that a (MinGW) system defines a macro for fileno. I tried to fix one of these silly MS warnings, but using 'fileno' as a method name seems to have been a bad idea.

Yes, it struck me as an odd thing for the header to do - and it seems to be peculiar to that flavour of mingw (which is from the “original” mingw, as it were) as the “other” mingw variants I have on that machine do not seem to have that “feature”...

>
> I'm afraid I can't build in that environment, so I'd like to ask you to try a fix:
>
> We can probably fix this by renaming our own 'fileno' method to 'filno' to avoid the name clash, see attached diff. Can you please try if this fixes the issue?
>
> Just in case '::fileno()' chokes as well, you can try to remove the '::' since we shouldn't have a conflict after the rename to 'filno’.

I’ll need to do it tomorrow, when I have access to the same machine - for consistency, if nothing else!

[imacarthur]

OK, tested with the patch (copied below) and that seems fine - built and worked OK with both the 32 and 64-bit mingw targets.

Went with "get_fileno" for the "replacement" name, as I felt that described what it was doing and would not collide with the macro #define.

I also had to remove the "::" from line 1360 to get this to compile.

 

///////////////////////////

diff --git a/fluid/fluid.cxx b/fluid/fluid.cxx

index 0c1647b48..b5e01f316 100644

--- a/fluid/fluid.cxx

+++ b/fluid/fluid.cxx

@@ -1353,11 +1353,11 @@ public:

   // returns fileno(FILE*):

   // (file must be open, i.e. _fpt must be non-null)

   // *FIXME* we should find a better solution for the 'fileno' issue

-  int fileno() const {

+  int get_fileno() const {

 #ifdef _MSC_VER

     return _fileno(_fpt); // suppress MSVC warning

 #else

-    return ::fileno(_fpt);

+    return fileno(_fpt);

 #endif

   } // non null if file is open

 

@@ -1501,7 +1501,7 @@ shell_pipe_cb(FL_SOCKET, void*) {

     shell_run_terminal->append(line);

   } else {

     // End of file; tell the parent...

-    Fl::remove_fd(s_proc.fileno());

+    Fl::remove_fd(s_proc.get_fileno());

     s_proc.close();

     shell_run_terminal->append("... END SHELL COMMAND ...\n");

   }

@@ -1537,7 +1537,7 @@ do_shell_command(Fl_Return_Button*, void*) {

   }

   shell_run_window->show();

 

-  Fl::add_fd(s_proc.fileno(), shell_pipe_cb);

+  Fl::add_fd(s_proc.get_fileno(), shell_pipe_cb);

 

   while (s_proc.desc()) Fl::wait();

 

[Bill Spitzak]

Can you #undef fileno?

I agree that working around Microsoft stupidities are best done with a local patch, and not unnecessarily changing the code that works on other platforms (ie don't rename fileno!)

--
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/20E0399E-036F-43AB-ABA9-9BA918D87E06%40gmail.com.

[imacarthur]

On Tuesday, 28 September 2021 at 08:45:16 UTC+1 Bill wrote:

Can you #undef fileno?

I agree that working around Microsoft stupidities are best done with a local patch, and not unnecessarily changing the code that works on other platforms (ie don't rename fileno!)

This is a slightly tricky one - we have a couple of compounded stupidities here; MS did a thing (which Albrecht dodged around) but that then collided with a stupid thing that the mingw32 folks did (presumably in their attempt to dodge so weirdness). And the underlying issue is that we had a class method called fileno that was then calling the "real" fileno - in the mingw32 case that meant we can't just #undef it, because our method then needs to use that macro expansion to get the actual operation...

The fix here is for us *not* to have a class method called fileno, basically, which is fine.

[Bill Spitzak]

Sounds correct. You are right the problem appears to be a variable called fileno, not the function called fileno.

--
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/3f0e889c-716e-4da9-bd80-4d1098aac0abn%40googlegroups.com.

[Albrecht Schlosser]

On 9/28/21 9:43 AM imacarthur wrote:

OK, tested with the patch (copied below) and that seems fine - built and worked OK with both the 32 and 64-bit mingw targets.

Thanks for testing.

Went with "get_fileno" for the "replacement" name, as I felt that described what it was doing and would not collide with the macro #define.

OK.

I also had to remove the "::" from line 1360 to get this to compile.

As expected.

I committed your patch in b6a69db9a67c9353343d16814cbf35c5e3f36946. Thanks.