Support for AIX

[michaelbaeuerle]

Tested on AIX 5.1 with FLTK 1.4 git revision b3b0512.
Compiler: GCC 3.4 (supports C99, but the system libc only has partial support).

The AIX mount point detection code is broken in FLTK 1.4:
The working FLTK 1.3 code is in "Fl_File_Browser.cxx" and activated by "defined(_AIX)".
It used a C90 style for-loop and with the comma two existing variables are initialized at the beginning.
In FLTK 1.4 this was changed to C99 style and the comma now has different semantics (defines two variables of type "int").

This patch should fix the problem:
patch_src_drivers_Unix_Fl_Unix_System_Driver.cxx.txt
With the patch the file "Fl_Unix_System_Driver.cxx" can be compiled.

I have not tested the fixed code yet, because the AIX 5.1 system libc does not provide the C99 "lround()" function.
As discussed with Albrecht, C90 is out of scope for FLTK 1.4, therefore I propose no patch for this problem.

At the weekend I can patch in a local lround() replacement to verify that Fl_File_Browser works as expected.

[Albrecht-S]

@michaelbaeuerle Thanks for yet another test and patch.

I modified your patch slightly, pulling the declaration of struct vmount *vp down into the else block where it belongs (for clarity). [This was not your fault, it should have been there in the first place.] Please correct me if I missed something.

Waiting for your test results...

[ManoloFLTK]

I would propose the attached patch to provide support for lround(double) under AIX.

lround-aix.patch

[Albrecht-S]

@ManoloFLTK @michaelbaeuerle Just my 2 ct to the proposed patch:

1. OK in general, but since lround() is currently used only on macOS (Cocoa related stuff) and in src/Fl_Anim_GIF_Image.cxx we could as well make this locally in Fl_Anim_GIF_Image.cxx, particularly because the result of lround() is always cast to another type. It looks like we could use round() with a proper cast instead of lround() in the first place. @wcout What do you think (as the author of Fl_Anim_GIF_Image?).

2. The macOS usage doesn't need to be changed anyway because all supported versions provide lround().

3. The patch itself (if used as proposed) should probably be guarded by a POSIX version check to avoid multiple definitions if newer AIX versions provide lround() - maybe only to prevent using the own inline definition rather than the system provided one. @michaelbaeuerle ?

FYI: I tested a similar patch on Linux with and w/o the keyword inline. Using inline prevents a compilation warning on my Linux system (multiple definition of lround'otherwise). However, I'm not sure what happens iflround()has been defined by the includedmath.hheader and subsequently overridden by theinline` definition. I assume it would use the inline version and finally call round().

[wcout]

OK in general, but since lround() is currently used only on macOS (Cocoa related stuff) and in src/Fl_Anim_GIF_Image.cxx we could as well make this locally in Fl_Anim_GIF_Image.cxx, particularly because the result of lround() is always cast to another type. It looks like we could use round() with a proper cast instead of lround() in the first place. @wcout What do you think (as the author of Fl_Anim_GIF_Image?).

Using round() should be no problem. Guess I was using lround() only because initially it needed no casts, but as compilers later got more and more picky about these things casts were added again.

[Albrecht-S]

@wcout Thanks for your confirmation and explanation why lround() was used in the first place.

All: I believe using round() in Fl_Anim_GIF_Image.cxx is the way to go. Using lround() + cast rather than round() + cast doesn't make much sense to me - unless I'm missing something essential.

This would also "fix" this issue.

[ManoloFLTK]

@michaelbaeuerle Could you, please, report here whether 1.4 builds now OK on AIX ?

[michaelbaeuerle]

Tested with local lround() replacement: The mount point list was still broken.
Because the FLTK 1.4 version of the code does not increment the variable "num_files", the result is always zero.
With this patch the mount point list works again:
patch_src_drivers_Unix_Fl_Unix_System_Driver.cxx.txt
The code block for NetBSD has the same problem and is fixed by this patch too.

In theory all code paths can still overflow "num_files", so that a negative value will be returned.

[michaelbaeuerle]

@michaelbaeuerle Could you, please, report here whether 1.4 builds now OK on AIX ?

round() is missing too.

[Albrecht-S]

With this patch the mount point list works again:
patch_src_drivers_Unix_Fl_Unix_System_Driver.cxx.txt

Note: this new patch is partially outdated, only the num_files++; statement needs to be inserted - AFAICT. Please see commit 1c91072. Note also that this commit is intentionally slightly different than @michaelbaeuerle's (old and new) patch(es).

[Albrecht-S]

Summary, current status:

@michaelbaeuerle wrote:

round() is missing too.
Tested with local lround() replacement: The mount point list was still broken.
[above supplied patch fixes file counting]

I applied the missing part (file counting) in 5666ec0. This provides the prerequisite for #944.

I believe this is all we can do if we don't want to add a round() implementation in our FLTK code. As Michael wrote, his AIX system works with his own lround() implementation - but this would now need a round() implementation after @ManoloFLTK replaced lround() with round(), which I agree to be the better choice.

Hence I believe we should now close this issue. Michael, is this OK for you?

[michaelbaeuerle]

Hence I believe we should now close this issue. Michael, is this OK for you?

Yes, please close if you don't want to add a replacement for "round()".

For my system I will test this replacement code:

static inline double round(double x) {
  return (x >= 0) ? floor(x + 0.5) : ceil(x - 0.5);
}

[Albrecht-S]

OK, I'm closing this issue now. round() is C99 and should be available on all our target systems. If it is not, then either the user must provide it, or we'd need a configure or CMake check to make sure that we really find the function. I don't intend to implement this though.

I'm closing this issue with the "fixed" label.