[Freeciv-Dev] Compaq Tru64 Unix Alpha platform - Building freeciv

[Davide Pagnin <nightmare@xxxxxxxxxx>]

        Hi All!

This is another mail! 
(And on a completely different topic!!!)

I've managed to compile freeciv on a Alpha platform with Tru64 unix 5.1A
(formerly Digital Unix).

I've already read that Ben Webb has the same platform (alpha) but
It was not clear if he used linux or Tru64 Unix.

Why I want to state this?
Well, we are in a feature freeze moment, with deep playtesting an
bugfixing,
and I hope that patches that can make easier building the game under
other platforms and unices are appreciated.

I've spotted some kind of common problem with the actual code in the
cvs,
and definitely they depend on the different architecture.
(I imagine that alla of you know that alpha processor are true 64 bit,
instead of 32 bit like Intel x86 processors)

I'll make summary of the most common type of problem:

1) when you feed an operand to a function of the print family, with
a format of %i or %d, gcc expects you feed a 32 bit integer, even 
with alpha platform, so it is necessary to explicitly cast to integer
or unsigned integer every pointer that has to be passed to such
a function. This applies also to particular case, like the passing of
a precision field.

Example:
size_t synlen = strlen(syn)
my_snprintf(prefix, sizeof(prefix), "%*s", synlen, " ");

size_t as the POSIX standard ask, is a machine dependant type,
with has a architecture variable dimension, in particular

for x86 architecture is defined as:
typedef unsigned int size_t (thus 32 bit)

for alpha architecture is defined as:
typedef unsigned long size_t (thus 64 bit)

This make the compiler issue a warning every time such a construct
is encountered.

There are 2 simple solution that I can see:
1) use unsigned int instead of size_t for variables that are
related to the length of a string
2) explicitly cast the size_t variable to unsigned int in the
printf function

Any strong opinion on how is the right (TM) way to resolve this issue?

2) When you use the XtPointer defined by Xt intrinsics, you definitely
use a pointer, but in the present coding style of freeciv, many time
it is necessary to convert such pointers to integers, and this is done
by casting them to int.

This make the compiler warn of a conversion from pointer to integer
of different size, because obviously pointers on alpha are 64 bit,
and int are 32 bit.

Example:
void races_toggles_callback(Widget w, XtPointer client_data,
                            XtPointer call_data)
{
  int index,j,race;
  ...
  index = (int)client_data;

A simple solution (but I thing is not clean), is to convert
such case to (long) instead of int, this make the trick both on
64 and 32 bit platforms. (But clearly doesn't work for 16 bit 
platforms, like amiga or old 286,386 and the like)

Another possibility is to have them convert by a type machine
dependant, like size_t or similar types, with I think is the
cleanest solution

3) Again over the XtPointer, this time there is the inverted 
conversion problem, from an integer to a pointer, there is the
same mismatch.

Example:
int what;
...
XtAddCallback (button, XtNcallback, pillage_callback, (XtPointer)what);

Ad again I thing that what has to be defined with a machine dependant
type.

4) Another annoying warning present in many file was the "char
subscript",
related to the family of function like isalnum, isspace, toupper,
tolower,
isdigit. Alla of this are defined as: isspace(int c) and the c parameter
has to be convertible to an unsigned char or be the macro EOF.

In the freeciv code, instead, it happens many time that signed char are
feed to the funcion, in particular they are part of string, and so they
are part of an array, this issue the warning, and I think that is more
a problem of gcc than of alpha architecture, nontheless I'll ask your
opinion on these "subscript" problem.

Example:
static bool is_ok_opt_name_char(char c)
{
  return isalnum(c);
}

Besides ignoring this type of warning, the only solution I foresee is 
an explicit cast to int of the c parameter, because it is not possible
to have the string defined as arrays of unsigned char. (We may also
cast to unsigned char, but is is longer and the function definition
ask for an integer)

5) There is a problem also with some asserts that eat a pointer as
the argument of they evaluation. I remember that some time ago Raimar
tried to convert a whole bunch of implicit pointer bool evaluation to
explicit, then all of this was undone, for some good reason.
Nontheless, I've spotted only 3 or 4 case in which alpha platform
require to change the actual code, so perhaps this is not a problem
for other reason. Feeding a pure pointer to an assert make the
compile warn of the convertion from pointer to integer of different
size (remember that int, the parameter of assert, in only 32 bit!).

Example:
assert(pplayer);

Solution, make the boolean evaluation explicit: assert(pplayer!=NULL);

6) The last problema I've found is a very unlike to happen one,
my problem is related to configure and readline library, the Tru64
Unix is somewhat non standard (or too standard! who knows?) and 
when asked to build with the readline library, it doesn't work
unless you use the termcap or the curses library.
The configure of freeciv handle this problem quite correctly, but
there is still a problem, the configure script search in the
termlib, termcap, curses and ncurses (in this order!) for the
presence of the initscr function, but when compiling readline need
the tgetent family of function the the initscr so finding the
support for initscr doesn't imply that the library support the
tgetent family of function, this has an insidious side effect,
because ncurses library do have support to initscr but do not
have support for tgetent, and beeing the last tried, it is the
one choose by configure, that fails to build for this reason.

I'm not a configure hacker, what I can say is that a solution can be
the change of the order, if this don't hurt for other reason,
a less stupid way of handling this can be to search for tgetent
family function instead of initscr (or for both). I'm open
to every suggestion on this point...

I hope that this feature freeze time is a good moment to have the time
to clean the code even from this warning for odd architecture (not so
odd in the end, because Itanium is approaching...).

And so I wait to know what solution do you think are the best for
the problems I've spotted, so that I can make a patch to clean the
cvs. (Perhaps also Ben will help in this...)

        Ciao, Davide

P.S. I can already send a patch with my clean build tree, but I've 
implemented mine style choices and I prefer to wait your opinion
before sending the patch, so that analyzing is made in a more
clean and theorical way.