Re: [AD] [PATCH] Solaris Fixes and dev notes

[Shawn Walker <binarycrusader@xxxxxxxxxx>]

On 6/6/05, Evert Glebbeek <eglebbk@xxxxxxxxxx> wrote:
> > There were a few small problems in the patch, I've corrected them, and
> > made one more change to make this even more robust. See attached
> > patch.
> 
> Could you elaborate a bit on what you changed?

in aclocal.m4, you had:
[allegro_sys_getexecname = yes], [allegro_sys_getexecname=no])]

instead of:
[allegro_sys_getexecname=yes], [allegro_sys_getexecname=no])]

The spaces before the equal sign of yes condition caused syntax errors
in generated configure script.

You misspelled Solaris in the comment for ALLEGRO_SYS_GETEXECNAME for
configure.in

> -      #ifdef ALLEGRO_HAVE_PROCFS_ARGCV
> 
> No, this is wrong. The pr_argv field is not portable to all systems, we
> need the configure check.

Ooops. My intent was that if pr_argv failed, to try the emulate using
pr_psargs step even for systems that have ALLEGRO_HAVE_PROCFS_ARGCV
defined.

> +      if (psinfo.pr_argv && psinfo.pr_argc) {
> 
> This should be ok, but I'm puzzled: surely, if pr_argv is not NULL it has
> to point to something? Why the extra check against pr_argc?

Because it can contain random memory garbage as I discovered last
night or it can be NULL. In the cases where it was NULL and in the
cases where it contained random garbage pr_argc was == 0. I managed to
simulate the condition where getexecname() returns only the executable
name without an absolute path, which caused the psinfo code path to be
hit.

Thanks,
-- 
Shawn Walker, Software and Systems Analyst
binarycrusader@xxxxxxxxxx - http://binarycrusader.blogspot.com/