Re: [rdesktop-devel] rdesktop patches

[Bob B. <bo...@zk...>]

On Mon, Jul 29, 2002 at 10:26:47AM +0200, Peter strand <pe...@ce...> wrote:
> >The first is a patch to make the using of variable
> >arguments to function-type macros consistent with the C99 standard.
> >This should continue to work with gcc, and was required to work with
> >Compaq C on Tru64 UNIX.  
> 
> The main version is quite different from the 19-9-0 version. I've tried 
> to fix all varargs I could find. Can you try the latest CVS version and 
> check if it compiles on your platform?

    Yes, it compiles, though not cleanly.  The varargs issue appears to
be cleaned up.

> >The second patch consistents of touch-up to
> >pointer casting that removed a lot of warning messages.
> 
> I've tried gcc -Wall, but as far as I can see, none of your changes 
> applies to the main version. There are still other warnings though...

    I get a couple of issues.  Note that I am using 'cc', the Compaq
C compiler, on Tru64 UNIX, which works much better on Alpha, and is in
many ways a more sophisticated compiler.  Note that I am not using the
-Wall equivalent, but just a default compile.

    I've attached two patches.  The first, rdesktop-rjb-1.patch, fixes
behavior that I think is definitely in error.  The affected lines of
code have undefined behavior that may not be portable between
C compilers.  It should be checked to verify that it implements the
desired behavior, since the original intent is not 100% certain to me.
I believe even if I got this wrong, an equivalent change should be made
to avoid this undefined behavior.

    The second patch, rdesktop-rjb-2.patch, simply clears up some
remaining pointer issues.  Not a big deal, but takes care of some
warning messages on my platform.  I'm one of those that believes code
should compile without any such messages.  To apply the patch cleanly,
apply after applying rdesktop-rjb-1.patch (both touch xwin.c).

    Lastly, are you aware that SCANCODE_CHAR_BACKSPACE is defined twice
in scancodes.h?  I was going to fix this, but I wasn't sure what the
desired fix would be.  It's unconditionally defined twice, so it's
always SCANCODE_KEY_291, never SCANCODE_KEY_15.  Since it's
SCANCODE_KEY_291 instead of SCANCODE_KEY_29, and SCANCODE_KEY_291 is
used nowhere else, I'd have to say that something is definitely going on
that I don't know about.  I believe the end result is that
SCANCODE_CHAR_BACKSPACE is not defined, though perhaps that matters
little, since it doesn't seem to be in use at all.  Care to enlighten me
as to what's going on?

-- 
Bob Bell <bo...@zk...>
-------------------------------------------------------------------------
 "MSN [the Microsoft Network] has a guy whose full time job is
  walking around rebooting NT Servers as they crash."
   -- Alex St. John, former Microsoft employee