Subject: gsfr

From: [email-masked]
Date: Fri, 13 Nov 1998 13:02:35 +0100

I want to do some experimenting for the GSFR soon so that
we can have it finished not too far in the future. There
is a proposal how to do it in the TODO file. I think it's
good, but I'd like to use an array of chars rather than ints
(ints may vary in size, chars do not).

Bye

Dominik
-- 
Dominik Vogt, Hewlett-Packard GmbH, Dept. BVS
Herrenberger Str.130, 71034 Boeblingen, Germany
phone: 07031/14-4596, fax: 07031/14-3883, [email-masked]
--
Visit the official FVWM web page at <URL:http://www.hpc.uh.edu/fvwm/>.
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: Fri, 13 Nov 1998 07:31:26 -0500 (EST)

> (ints may vary in size, chars do not).

	No, it's an array of 1-bit bitfields (these are of a
pretty constant size, too). This is definitely the way to go.

	There are a couple of questions we should kick around,
though. 

	1) Chuck has them grouped into a struct within a union.
Is this really necessary? It doesn't seem so to me.

	2) I feel strongly that we need to divorce window *style*
flags from window *state* flags. The one type has little to do
with the other, and the necessitly of keeping subsets of two
different structures artificially aligned is confusing and
error-prone. (It's also one reason the flags word filled up too
quickly).
	If we enclose the flags within a struct or union, there
need to be two of them: one for style flags, and one for state flags.

	3) Some people like the idea of masking the bitfield
manipulation with a set of macros, e.g.,

	SetSticky(Win)

would do something like

	Win->Sticky = True;

	I kind of like this idea. Greg Badros went a step further
with SCWM, and defined a derived data type called PackedBool, or
something like that, which is a single bit field. We don't need to
go that far, but it wouldn't hurt, either.

- Bob

---

From: [email-masked]
Date: Fri, 13 Nov 1998 14:14:54 +0100

> Is this really necessary? It doesn't seem so to me.

It's easy to duplicate with memcpy as a union with a character
array.

> need to be two of them: one for style flags, and one for state flags.

The reason for ONE union is compatibility with the old code.
Otherwise we'd have to scan much more code for possible errors.

> 	I kind of like this idea.

I don't like it at all. Marcos used in this way tend to make
you think they do something different than they do, they make
the code less readable, harder to change, may produce lots of
unnecessary code. Just have a look at the SET_MANAGER macro
in modules/FvwmIconMan/readconfig.c and you know what I mean.
There's much code duplication caused by this macro. But do try
to change that. I couldn't. You may object that this is not the
case with simple macros like the on above. But then you don't
need it at all.

I have seen a lot of macros in my life. Macros that contain actual
code almost never help you but can cause many problems (remeber
they fall through the syntax checker in C).

Bye

Dominik

---

From: Austin Donnelly <[email-masked]>
Date: Fri, 13 Nov 1998 13:50:50 +0000

> (ints may vary in size, chars do not).

The reason for using ints (or better, something word-sized, eg a long
on most architectures) is that loading a word from memory is often
faster than loading a single byte.  This is especially true for RISCy
CPUs, but these days probably on ix86 as well.

Can autoconf give us a word_t and the number of bits wide it is?

Austin

---

From: [email-masked]
Date: Fri, 13 Nov 1998 15:19:21 +0100

> CPUs, but these days probably on ix86 as well.

OK, but the flags are copied only rarely. Typically once for every
window.

Bye

Dominik

---

From: Greg Badros <[email-masked]>
Date: 13 Nov 1998 07:42:09 -0800

Bob Woodside <[email-masked]> writes:

<snip>

> go that far, but it wouldn't hurt, either.

The PackedBool abstraction makes it really easy to choose to use a long
instead of a single bit field, or whatever makes the most sense for a
given platform.  Also note that beginning of Scwm's window.c has some
functions:

ResetCommonFlags
ResetAllFlags
CopyCommonFlags
CopySetCommonFlags
CopyAllFlags
CopySetAllFlags

that are used to accomplish what the previous code did with a bunch of
bit manipulations.  The above functions are written in the most obvious
way-- they are not a performance bottleneck, so it's not worth
sacrificing portability or readability. 

Though some of Scwm's related code has changed or been extended, the
mapping back to the fvwm2 source is probably straightforward enough to
understand w/o too much trouble.

Greg

---

From: [email-masked] (Paul D. Smith)
Date: 13 Nov 1998 11:10:34 -0500

>> CPUs, but these days probably on ix86 as well.

  dv> OK, but the flags are copied only rarely. Typically once for every
  dv> window.

I don't think this argument is really germane.  The compiler is free to
implement bitfields however it likes; we will have to trust it to do at
least as good a job as we would do (use the appropriate native type for
fastest service, etc.)

Also, the whole "copying" thing doesn't matter; when you invoke
memcpy() it cannot know what the C type of the memory is.  It will move
the memory around as efficiently as possible.

Ditto for whole-structure copying, if that's what you mean; we must rely
on the compiler to implement that as efficiently as possible regardless
of the types involved.

There was a lot of discussion of this a while back; I suggest interested
parties examine the archives.  I can't remember the reasoning for the
union offhand.

One use would be if you wanted to treat them as integers at some point;
for example, passing them to modules which uses "packets" consisting of
ints.  Or maybe to print them out in blocks, instead of individually.
But we never do either of these.  So I'm not sure.

-- 
-------------------------------------------------------------------------------
 Paul D. Smith <[email-masked]>         Network Management Development
 "Please remain calm...I may be mad, but I am a professional." --Mad Scientist
-------------------------------------------------------------------------------
   These are my opinions---Nortel Networks takes no responsibility for them.
--
Visit the official FVWM web page at <URL:http://www.hpc.uh.edu/fvwm/>.
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] (Paul D. Smith)
Date: 13 Nov 1998 11:33:58 -0500

>> Is this really necessary? It doesn't seem so to me.

  dv> It's easy to duplicate with memcpy as a union with a character
  dv> array.

As I mentioned before, I don't follow this.  memcpy() takes a void
pointer and a length (sizeof...); it can't know or care _what_ the C
structure of the pointed-to memory is.  It just moves memory.  It will
be no more or less efficient at it whether the bitfields are unioned
with an array of ints, or an array of chars, or anything else.

  dv> The reason for ONE union is compatibility with the old code.
  dv> Otherwise we'd have to scan much more code for possible errors.

Note this is the GSFR.  We've long been planning this as a somewhat
major project (which is why we haven't done it yet).  I think we should
choose the best implementation and go with it, and not rely on
compatibility.  If it takes longer to find the problems associated, so
be it; the important thing is to get it right.  I don't know whether
there are good reasons to split or not, but if there are we should do
it.

Also, note there's no need to scan the code for errors; let the compiler
do it! :).  We're changing the C structure, presumably renaming the
fields, changing their types, etc.  The compiler will faithfully point
out any parts of the code we missed ;).

  >> SetSticky(Win)

  dv> I don't like it at all. Marcos used in this way tend to make
  dv> you think they do something different than they do, they make
  dv> the code less readable, harder to change, may produce lots of
  dv> unnecessary code.

I don't have strong feelings, either way, but I think I disagree with
most of this reasoning :).

The macro here clearly sets the sticky attribute for window win.  Now,
the entire point of using a macro is you don't _know_ (or have to know)
how that happens.  As long as that's what it does (and _only_ what it
does), then who cares how much code it hides?  Macros like this are (or
should be) simply faster, more efficient ways to implement data-hiding
that you'd probably do in a function if it weren't so slow.

Second, this code is just as readable to me, if not moreso, than
the win->Sticky = 1 variant... that leads you to wonder: what if I set
it to 2?  Which is obviously illegal.

Third, I don't see how it's harder to change; just the opposite!  If we
were using those macros all along, for example, we could do what Dan
suggests and make two different structures instead of one and change
_NO_ code!  In fact, the GSFR itself might be accomplished in its
entirety without needing to change _any_ C code, just the header file!
The entire point of macros like this is to concentrate details into one
place to allow the details to be changed later, invisibly, without
modifying all the code that uses the feature.

  dv> Just have a look at the SET_MANAGER macro in
  dv> modules/FvwmIconMan/readconfig.c and you know what I mean.

I haven't looked at it, but I know where you're coming from.  I've seen
_MANY_ examples of the C preprocessor used inappropriately.  In
particular, when it's used as a poor-man's inline function it can be
gross.  But, simple things like this don't bother me.

  dv> There's much code duplication caused by this macro. But do try to
  dv> change that. I couldn't. You may object that this is not the case
  dv> with simple macros like the on above. But then you don't need it
  dv> at all.

Not so, for the reasons I've outlined above.  There are many advantages
to implementation-hiding via macros, even if the macros expand to
something just as easly written by hand.

  dv> I have seen a lot of macros in my life. Macros that contain actual
  dv> code almost never help you but can cause many problems (remeber
  dv> they fall through the syntax checker in C).

??  Not so.  Obviously the compiler will be checking syntax on the
post-processed code, not the preprocessed code.

Perhaps you mean other kinds of syntax-checkers, like lint?  I don't
remember whether lint preprocesses first or not, but anyway, I don't
think it's used much anymore since the advent of more robust warning
features in GCC for example.

---

From: [email-masked]
Date: Mon, 16 Nov 1998 09:57:43 +0100

> with an array of ints, or an array of chars, or anything else.

OK

> there are good reasons to split or not, but if there are we should do

We rewrite the code in more than one step. This makes life a lot easier.

> do it! :).

Semantic errors!

> that you'd probably do in a function if it weren't so slow.

It 'clearly sets the sticky attribute for window win' until there is
a problem that you can easily solve by adding another line to the macro.
Then you'll have this line 9 times out of ten in places where it isn't
needed.

> it to 2?  Which is obviously illegal.

With bitfields it won't even compile.

> modifying all the code that uses the feature.

I knew that reasoning would come up. If we want to do the GSFR by
changing macros, we have clearly screwed up the design before. This
way macros are an excuse for writing dirty code. There's plenty of
this kind already (did you ever look at style.c?) and I think we can't
we can't afford misdesign any more.

> gross.

The macros I have seen (simple value/flag definitions excluded) are
used in an abusive way about 90% of the time. And we don't even have
*one* authority on which style of code/patches makes it into CVS.

> But, simple things like this don't bother me.

Neither do they bother me. My fear is that 'simple things' will become
more complex over time up to the point where any attempt to rewrite
them may fail (*do* look at SET_MANAGER).

> post-processed code, not the preprocessed code.

How about type checking? The compiler won't notice you use chars where
integers are expected.

> features in GCC for example.

Oops. Lint (at least some of the enhanced lints) is much better than
any compiler can be.

---

From: [email-masked]
Date: Mon, 16 Nov 1998 12:13:08 +0100

Whatever the details will be in the end, there seems to be
a consensus that the flags will be implemented with bit fields.
A logical first step would be to code this in a single struct,
not a union until the benefit of the union is proven.

Bye

Dominik

---

From: Dan Espen <[email-masked]>
Date: Mon, 16 Nov 1998 08:56:16 -0500

> we can't afford misdesign any more.

Hey, wait a minute, I put those macros in style.c!

Well, you may have a point.  I put those macros in as a first step to
boiling the code down a little.  It sorely needs to be table driven.

I was just trying to make the actual structure of the parsing a little
clearer.

Take a look at the same code without the macros, and I think you will
appreciate the macros a little more.

Don't get  me  wrong, I'm not  getting involved  in  the GSFR  with or
without macros controversy.

--
Dan Espen                                Phone: (732) 699-5570
444 Hoes Lane  Room RRC 1C-214           E-mail: [email-masked]
Piscataway, NJ 08854
--
Visit the official FVWM web page at <URL:http://www.hpc.uh.edu/fvwm/>.
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: Mon, 16 Nov 1998 15:52:57 +0100

> appreciate the macros a little more.

Hmm, in my eyes it's easier to understand without the macros.
The operation is trivial but the macro forces me to look somewhere
else.

Bye

Dominik

---

From: [email-masked] (Paul D. Smith)
Date: 16 Nov 1998 13:50:45 -0500

%% Regarding Re: gsfr; you wrote:

  >> major project (which is why we haven't done it yet).  I think we
  >> should choose the best implementation and go with it, and not rely
  >> on compatibility.

  dv> We rewrite the code in more than one step. This makes life a lot
  dv> easier.

OK, that's fine.  But I repeat: the GSFR has been a long time in coming
and we should do it _right_, and not worry too much about how much fvwm
code might break.  That's why we're waiting until the next release: we
expect things to break.

  >> Also, note there's no need to scan the code for errors; let the
  >> compiler do it! :).

  dv> Semantic errors!

I guess I'm thick, but I just don't see what kind of semantic errors
you're envisioning being a problem by splitting one structure (which
doesn't even exist yet!) into two.  You're going to have to change all
that code anyway for GSFR.  And any code you _didn't_ change would fail
to compile, whereupon you could examine the code and ensure that it was
both syntactically _and_ semantically correct.

  dv> It 'clearly sets the sticky attribute for window win' until there
  dv> is a problem that you can easily solve by adding another line to
  dv> the macro.  Then you'll have this line 9 times out of ten in
  dv> places where it isn't needed.

That is obviously terrible code.  If people are going to write bad code,
then they'll write bad code.  I'm not sure we should forgo any possible
advantages in readability and maintainability this might give us because
of that.

But this, definitely, is the crux of the argument so far.

  >> the win->Sticky = 1 variant... that leads you to wonder: what if I
  >> set it to 2?  Which is obviously illegal.

  dv> With bitfields it won't even compile.

I'm talking about reading and comprehending the code.  Certainly it
won't compile, but who's going to try to change it and compile it just
to see?  Anyway, I'm just pointing out that between these two:

  SetSticky(win);
  win->sticky = 1;

it's not obvious to me that the second is superior, in terms of
understanding what it does, than the first.

Obviously none of this holds if SetSticky() _doesn't_ do that, or does
more, but that's another argument (see above).  Maybe it's my happy
experience so far, but I typically take macros and functions at face
value and don't bother going to see what they _really_ do... unless it
seems likely there's something wrong with them.  That makes them a net
win in ease of understanding for me :).

  >> The entire point of macros like this is to concentrate details into
  >> one place to allow the details to be changed later, invisibly,
  >> without modifying all the code that uses the feature.

  dv> I knew that reasoning would come up. If we want to do the GSFR by
  dv> changing macros, we have clearly screwed up the design before.

Well, obviously the current style code _IS_ broken, that's why we have
to rewrite it.  My point is that it could have been rewritten without
changing any code _if_ some abstraction from the fundamental types were
used.

Also, I'm not sure I agree with the argument that implementation- and
data-hiding is useless because if the design was right it wouldn't be
necessary :).  In changing environments and requirements, the design may
need to change.  The ability to do this with minimal disruption to code
which doesn't need to know about the change is the hallmark of a good
implementation.

  dv> The macros I have seen (simple value/flag definitions excluded) are
  dv> used in an abusive way about 90% of the time.

Wow, you've had bad experience.  I've had the opposite, fortunately:
most of the time I see macros they're useful and used properly.  There
are exceptions but they're not many, and squashed with prejudice
wherever possible.  Note that your parenthetical statement leaves out
about 90% of the macros I've seen in my life, though :)

  dv> And we don't even have *one* authority on which style of
  dv> code/patches makes it into CVS.

We have better than that, we have many people with "authority" :).

  >> post-processed code, not the preprocessed code.

  dv> How about type checking? The compiler won't notice you use chars
  dv> where integers are expected.

???  Of course it will.  The compiler does all this _after_ the macro
has been completely expanded.  The compiler doesn't even _SEE_ the code
until after the preprocessor is done with it.

There is no check that the compiler will or will not make differently on
code using a macro than it would make on the same code with the macro
expanded (since the compiler is seeing the code with the macro expanded).

  >> features in GCC for example.

  dv> Oops. Lint (at least some of the enhanced lints) is much better than
  dv> any compiler can be.

But, most lint's don't work with ANSI code/prototypes.  Anyway, this
argument is moot because I just checked the lint man page and is sez:

  | lint runs the C preprocessor as its first phase,

so lint will _also_ not be fooled by macros, just like the compiler.

---

From: [email-masked]
Date: Tue, 17 Nov 1998 13:08:59 +0100

> both syntactically _and_ semantically correct.

E.g. you copy the struct with the style flags but forget to copy the
state flags. Probably done only a few times but may be hard to find.

> But this, definitely, is the crux of the argument so far.

In this case I don't see better readability. And it should be
unnecessary to maintain that if the design is done properly.

> to see?

Oops, are you saying people will do such changes and commit them without
compiling first?

>  Anyway, I'm just pointing out that between these two:
> 
>   SetSticky(win);
>   win->sticky = 1;
> 

I don't trust names. They may suggest things that are not done.
This is a frequent cause of confusion with macros.

> ...

> which doesn't need to know about the change is the hallmark of a good

I don't say it's useless. I just hate macros to do it.

> wherever possible.

I am willing to give it a try, but I'm sceptical. In this case it
may be reasonable after all.

> We have better than that, we have many people with "authority" :).

That's no advantage in respect to coding style. Many people ==>
different styles ==> noone bothers about the style of patches (?).

>... 

#define SETCHARPOINTER(c, p) { p = (char *)&c; }

{
  int i;
  int *pi;

  SETCHARPOINTER(i, pi);
}

That's what I mean.
 
> so lint will _also_ not be fooled by macros, just like the compiler.

Other lint derivates do a check on preprocessor directives if you tell them.

Bye

Dominik

---

From: [email-masked] (Paul D. Smith)
Date: 17 Nov 1998 11:32:49 -0500

dv> E.g. you copy the struct with the style flags but forget to copy the
  dv> state flags. Probably done only a few times but may be hard to find.

Hmm.  I guess I'd need to see the details of implementation to decide
how likely I think this is.

  >> won't compile, but who's going to try to change it and compile it just

  dv> Oops, are you saying people will do such changes and commit them
  dv> without compiling first?

No.  Again, I'm talking about _reading_ and _comprehending_ the code.
Not changing the code.

I'm reading a function in my editor, to see what it does.  I look at the
line.  I see the value set to 1.  "1" is a magic number: there's nothing
_inherent_ in the line that tells me what it means.  If I happen to know
that "win->sticky" is a bitfield of size 1 bit, I can infer that turns
on a boolean.  But maybe it's not; maybe I'd get a different "style" of
stickiness if I set it to 2.  I don't know.

I shouldn't have to change the code and compile it and get a compilation
error in order to understand what it does!  (Obviously that's silly; of
course I'd go look up the struct definition--but still, if the code is
well-written I wouldn't need to even do that to make a very accurate
guess as to what's going on.)

I suppose another way to fix this would be to change the fieldname to:

  win->isSticky = 1;

or something, so the name implies a boolean.

  dv> I don't trust names. They may suggest things that are not done.
  dv> This is a frequent cause of confusion with macros.

???

The same can be said of functions, of course.

Maybe it's just my experiences, but I prefer to trust the code as
written, including function and macro names.  If something turns out to
violate that trust, _then_ I swear a blue streak and complain bitterly
(or fix it myself :).

  dv> That's no advantage in respect to coding style. Many people ==>
  dv> different styles ==> noone bothers about the style of patches (?).

That's a choice we can make ourselves.  It's certainly not unheard of
for there to be coding guidelines in opensource development.

Anyway, it's one thing to be picky about brace placement, comment
formatting, etc. and quite another to be strict about modifications to
significant code areas, like GSFR.

  dv> #define SETCHARPOINTER(c, p) { p = (char *)&c; }

  dv> That's what I mean.

Anyone can break any kind of error checking with casts.  And anyone can
write bad macros... just as they can write bad code otherwise.

There are enough eyes looking at the code that I don't feel worried that
this sort of thing will become a problem.  Maybe I'm just an optimist :)