| Re: [AD] [PATCH] Solaris Fixes and dev notes |
[ Thread Index |
Date Index
| More lists.liballeg.org/allegro-developers Archives
]
On Sunday 05 June 2005 08:07, Shawn Walker wrote:
> Attached is a patch that fixes the following issues:
Sorry I didn't respond to this problem earlier, I might have been of some
help!
> 1) The System V procfs calls method that Allegro was using to get the
> executable name was segfaulting in the method _unix_get_executable_name.
> Specifically, for whatever reason psinfo.pr_argv is undefined on Solaris
10.
> The code assumes that is not and as a result segfaults. I have instead
> placed a properly #ifdef'd solution at the beginning of this routine that
is
> significantly less code, faster, and more reliable. I leave it as an
> exercise to someone more involved in this project to properly check
psinfo
> before trying to access data members as an array, etc.
The configure script should check the psinfo struct for compatibility and
not set ALLEGRO_HAVE_SV_PROCFS if it doesn't work. This is obviously
broken somehow.
Anyway,
+ /* The System V way doesn't work on Solaris */
+ #if ((defined(sun) || defined(__sun__)) && defined(__svr4__))
+ s = getexecname();
+ if (s) {
+ do_uconvert (s, U_ASCII, output, U_CURRENT, size);
+ return;
+ }
+ s = NULL;
+ #endif
The comment is misleading - it used to work with older versions of Solaris
as far as I know (I recall checking it when I changed
_unix_get_executable_name()). I no longer have acces to any Solaris system
(safe SourceForge's compile farm) so I can't reconfirm that.
Actually, the
#ifdef ALLEGRO_HAVE_PROCFS_ARGCV
if (_find_executable_file(psinfo.pr_argv[0], output, size))
return;
lines were there specifically for Solaris. I guess we should add a check
for pr_argv being NULL though.
That said, getexecname() looks like a very convenient function to use in
this case! Rather than checking for any of the sun preprocessor directives
(which are unreliable, as I recall) we should have the configure script
check if the function exists - afterall, we don't want to know if we're on
Solaris, but if getexecname() works!
I'm a little bit concerned though: the manpage,
http://docsun.cites.uiuc.edu/sun_docs/C/solaris_9/SUNWaman/hman3c/getexecname.3c.html,
suggests that it isn't always reliable, so we should at least check if it
the return value makes sense or not.
> 2) In several key cases XKeysymToString was being used to directly pass
> arguments to other routines, without checking the return value first.
This
> is bad. XKeysymToString can return NULL as a value. I only fixed the
places
> that were causing Allegro to segfault for me, not all of them. I noted
that
> the Allegro code was already checking this in some places, but not in
> others. This should be addressed.
I agree. Do you think you can make a (seperate) patch for that?
> 3) I was unable to compile on Solaris since there is no GFX_MODEX support
> available, and the #ifdef checks being used are rather pointless since
I'm
> told the GFX_MODEX is always defined. So, they were changed to
GFX_HAS_VGA
> instead per the recommendation of one of the other list members here.
GFX_HAS_VGA is, I think, supposed to be internal only. The problem should
be addressed by defining the modex* functions on all UNIX systems (bleh)
since GFX_MODEX `exists' on all UNIX systems as well.
Evert