Subject: FvwmIdent on Tru64 reports interesting X and Y coordinates...

From: Jeremie Petit <[email-masked]>
Date: Fri, 4 Feb 2000 13:38:20 -0000

Hello,

		  I could successfully run fvwm2 on my Tru64 machine, and
discovered that FvwmIdent used to
		report some really strange X/Y/boundary width probably
related to the size of ints and longs etc.

		  Has any one else noticed that?

		Regards,

			Jeremie - On the long road to customize the new
fvwm2
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: [email-masked]
Date: Fri, 4 Feb 2000 09:00:05 -0500 (EST)

> 		  Has any one else noticed that?

Just noticed it:  Tru64 Unix 5.0 / fvwm-2.3.13

Specifically:

X, Y, Boundary Width have rather large values.  My display is 1600x1200, but
those values (eg, 858993462521145802772) are rather ridiculous!

-- 
Larry Gensch                                          [email-masked]

                  "May I have ten thousand marbles, please?"

---

From: Dan Espen <[email-masked]>
Date: Fri, 04 Feb 2000 09:36:15 -0500

> those values (eg, 858993462521145802772) are rather ridiculous!

Could be even a little more ridiculous
because the area the border width is put into is only 10 bytes wide.
You might be seeing more than one field run together.

Could we get some more info?
In FvwmIdent around line 305 could you add this:

fprintf(stderr,"Border Width sent %ld\n",body[10]);

That should show whether the number sent by fvwm is right.

Heres how I think it works:

The numbers come from an M_CONFIGURE_WINDOW packet sent by fvwm.
The packet is built in fvwm/module_interface.c by the
CONFIGURENEW macro:

	    (unsigned long)(sizeof(unsigned long)),\
            &(*(_t))->boundary_width,\

Bob,

  Isn't this code saying that boundary_width is stored as a long
in the FvwmWindow structure?  (Its an int.)
Does "make_new_vpacket" handle the conversion of ints to longs?
I don't quite follow it.  Yet.

--
Dan Espen
444 Hoes Lane  Room RRC 1C-214           E-mail: [email-masked]
Piscataway, NJ 08854                     Phone: (732) 699-5570
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Bob Woodside <[email-masked]>
Date: Sat, 05 Feb 2000 14:43:16 -0500

Dan --

	Sorry it's taken so long for me to get around to reading this. Sadly, I
haven't had much free time for working on FVWM lately. Just keeping up
with the mail has been a big deal.

> in the FvwmWindow structure?  (Its an int.)

	Yes, unfortunately. When I wrote the make_new_vpacket code for the GSFR
I was blithely assuming that everything pointed to by the CONFIGARGS
macro was a long, based on this snippet from the old make_vpacket, where
it's actually moving the args into the packet:

	  for (; num > 0; --num)
	    *(bp++) = va_arg(ap, unsigned long);

> I don't quite follow it.  Yet.

	No. The purpose of make_new_vpacket was to allow us to build an
arbitrary structure which the receiving module would reference via a
structure definition in vpacket.h, rather than referencing variables via
the old "body[6]" style syntax - all this so we could pass the new GSFR
flags structure. So make_new_vpacket just moves in fields for the length
you specify, with no notion of data types. The only special thing it
does is to round the total packet size up to a multiple of the size of
an unsigned long, for the benefit of the sending/receiving code.

> I would think that more than FvwmIdentify is going to be affected.

	Much more - everyone that looks at the M_CONFIGURE_WINDOW packet is
potentially affected on 64-bit machines, and at least has to be looked
at.

> I think the code in question should be more like:
> 
> Before:
>             &(*(_t))->title_g.height,\
> After:
>             (int)(sizeof(_t->title_g.height)),\
>             &(*(_t))->title_g.height,\
>             (int)(sizeof(_t->boundary_width)),\

	Not quite...

	c/(int)(sizeof(_t->title_g.height))/(unsigned
long)(sizeof(_t->title_g.height))

etc.

	The length arguments passed to make_new_vpacket must be unsigned longs
- the field-length argument is only used by make_new_vpacket, it doesn't
go into the packet.

	The problem is that the fields are getting stuffed into the packet with
an incorrect length, which would be easy enough to fix, in the way you
suggested above (with my correction). Unfortunately this would still
leave us with the problem that everyone expects these fields to be long
ints in the packet, and there's no provision for making this conversion,
so we'd still have the packet out of sync with what the modules expect.
Actually, it would be more so, since all the things that really are
longs, and are currently aligned properly, would now be misaligned as
viewd by the modules.

	This is true no matter whether the modules use the old "body[n]"
technique or use the ConfigWinPacket struct in vpacket.h; since I was
stupidly assuming that everything in this packet was a long (except for
the new window_flags struct, which was the whole reason for this
change), the ConfigWinPacket struct is incorrect as well.

> 
> Pass the size args as ints, shorts or chars would work too.
> Get the value for sizeof from the field itself.
> 
> Or perhaps, just cast everything to a long in the macro.

	Unfortunately, since the actual fields are being passed by pointer
reference, we can't play games with casting the actual data args (which
I believe is what the old make_vpacket did). But something that has the
same effect may be the best way to go.

> 
> But this won't work by itself, more changes are needed.
> 
> (I'm just trying to convey the problem at this point.)

	I'll have to give this some thought. Several things come to mind that
we might try.

	1) Change all of the FvwmWindow int items that are sent in the
M_CONFIGURE_WINDOW packet to longs. This would require a good bit of
double checking for new type mismatch problems in the core fvwm code,
but the packet structure would be back in sync and the modules should
not need changing.

	2) Replace the CONFIGARGSNEW macro technique of invoking
make_new_vpacket with a bit of code (or wrap the call with a bit of
intermediate code) that would convert all the int items to longs before
calling make_new_vpacket. This may be the best way to go.

	3) Redefine the make_new_vpacket API so that there is some way of
telling it that a particular argument is an int that needs to be forced
to a long. I'm not too wild about this one - it could get really ugly.

	4) Do a major investigation of all the modules that use
M_CONFIGURE_WINDOW to see how much trouble it would be to  fix them to
use the packet as it would be sent with the correction above to use the
actual lengths of the int fields. The problem I see with this approach
is that it would definitely break the old binary compatibility that
several people spent a lot of time and effort to try to maintain (i.e.,
the idea was that everything in the packet *before* the new window_flags
struct was supposed to continue to look just the way it always did).

	Now if only I had one of those Alphas to test on....  :-)

Cheers,
Bob

---

From: Dan Espen <[email-masked]>
Date: Mon, 07 Feb 2000 14:41:42 -0500

> with the mail has been a big deal.

No problem, being busy is good.

> > Heres how I think it works:
...
> - the field-length argument is only used by make_new_vpacket, it doesn't

Thanks Bob, I think I'm now clear on whats going on.

I still   don't see why the  length  arguments themselves   have to be
unsigned longs, their value is never large.  Not that I think it would
save much to make them smaller.

One thing I'm not clear on, are you going to fix this?
You sounded like you were formulating a solution.

It wouldn't make any sense for us all to work on it.

---

From: Bob Woodside <[email-masked]>
Date: Mon, 07 Feb 2000 17:22:15 -0500

> save much to make them smaller.

	Well, I doubt we need them to be that big, that's just the way I coded
the function. I think I had long ints on the brain that day.

> It wouldn't make any sense for us all to work on it.

	Well, since I messed it up in the first place, I think it would be the
neighborly thing for me to fix it.  :-)

	I'm just trying to find some time to work on it. I'll see what I can do
in the next couple of days. Otherwise, I won't be able to look at it
till next week.

	My thinking now is to take the approach (# 2 in my previous post) of
wrapping the make_new_vpacket call with a little function that will make
long ints out of those fields that need it. This will put the
M_CONFIGURE_WINDOW/M_ADD_WINDOW packets back the way everybody used to
expect them, and will involve the least code change.

Cheers,
Bob