shimoda@infohh.rmi.de (Markus Schmidt) (02/08/89)
Hi!
Every time using a Wait-GetMsg-pair I thought how to do it
best way. Now I think I got it:
while (1) {
while (!(imsg= GetMsg(win->UserPort)))
Wait (1<<win->UserPort->mp_SigBit);
... select imsg->Class and break when ended ...
}
I think that should ship around all "oddities" like multiple
Messages arriving or NULL-msges arriving?!
Any comments?
Markus dillon@POSTGRES.BERKELEY.EDU (Matt Dillon) (02/10/89)
:Every time using a Wait-GetMsg-pair I thought how to do it
:best way. Now I think I got it:
:
:while (1) {
: while (!(imsg= GetMsg(win->UserPort)))
: Wait (1<<win->UserPort->mp_SigBit);
: ... select imsg->Class and break when ended ...
:
:}
:
:I think that should ship around all "oddities" like multiple
:Messages arriving or NULL-msges arriving?!
:
:Any comments?
:
:Markus
Yes, that's probably the worse way to do it. Well, not the worse,
but it's not the best coding style I've seen (you asked for comments!).
Use something like this if you are handling only a single source of
events (i.e. one window's IDCMP): Note that since all messages are
processed before the next Wait(), you can safely do the Wait().
while (notquit) {
Wait(1 << win->UserPort->mp_SigBit);
while (mess = GetMsg(win->UserPort)) {
/* process message */
ReplyMsg(mess);
}
}
On the otherhand, if you are waiting for multiple events, the logical
extension works as well. Again, the assumption is that you handle
*everything* related to the signal bits in question before going back
to the Wait().
while (notquit) {
mask = Wait(mask1 | mask2 | mask3);
if (mask & mask1) { /* say this is for your intuition window */
register struct IntuiMessage *mess;
while (mess = GetMsg(win->UserPort)) {
/* process all messages */
ReplyMsg(mess);
}
}
if (mask & mask2) {
/* etc... */
}
if (mask & mask3) {
/* etc... */
}
}
The above works fine in the worst case: (1) A message comes in while
we are in the GetMsg() loop, causing a (harmless) spurious signal, and (2)
A message comes in just after we exit the GetMsg() loop but before the Wait(),
in which case we still get a signal. Also, note that the construction of
the code fragment allows you to easily declare very-local variables within
each if() block ... a much more modular coding practice as well as helpful
to the compiler's code generation.
An example of INCORRECT code:
while (notquit) { /* INCORRECT CODE */
Wait(1 << win->UserPort->mp_SigBit);
msg = GetMsg(win->UserPort);
/* process */
ReplyMsg(msg);
}
Will NOT work because it is possible to get into two crash situations.
(1) Several messages get queued to you quickly (before Wait() can clear the
signal bit), then Wait() clears the signal bit, you process just one of the
messages and go back to the Wait() ... the Wait() locks up the computer until
the next message comes in even though there is already one left on the port.
(2) You get a spurious signal and GetMsg() returns NULL (no messages on the
port). You must always check that GetMsg() returns a non-null message
pointer. In the examples of correct code, I have a while (mess = GetMsg...)
and thus the while() takes care of this check.
-Mattrokicki@polya.Stanford.EDU (Tomas G. Rokicki) (02/10/89)
>:Now I think I got it: >:while (1) { >: while (!(imsg= GetMsg(win->UserPort))) >: Wait (1<<win->UserPort->mp_SigBit); >: select imsg->Class and break when ended >:} This is *perfectly* fine. while(1)/break is a general looping construct. while(notquit) is fine, too. > Yes, that's probably the worse way to do it. Well, not the worse, >but it's not the best coding style I've seen (you asked for comments!). First, it's worst, not worse. (What I hear about Berkeley producing only bit pushers may be true.) Secondly, his code *works*, and the coding style is one I happen to agree with. Matter of taste, Matt. > while (notquit) { > Wait(1 << win->UserPort->mp_SigBit); > while (mess = GetMsg(win->UserPort)) { > /* process message */ > ReplyMsg(mess); > } > } Equivalent, *except* that you have a wait at the top that's not guarded by a `while (GetMsg())' loop. Fine, perhaps, depending on how you enter the loop and what operations you allow inside the loop (what those subroutines might do to the signals, you know) but conservative programming techniques never hurt anyone. > while (notquit) { > mask = Wait(mask1 | mask2 | mask3); > > if (mask & mask1) { /* say this is for your intuition window */ > register struct IntuiMessage *mess; > while (mess = GetMsg(win->UserPort)) { > /* process all messages */ > ReplyMsg(mess); > } > } > if (mask & mask2) { > /* etc... */ > } > if (mask & mask3) { > /* etc... */ > } > } Not very fair; mask1 can dominate, causing problems later. Something like int msgseen ; while (notquit) { msgseen = 0 ; if (msg = GetMsg(port1)) { msgseen = 1 ; } if (msg = GetMsg(port2)) { msgseen = 1 ; } if (msg = GetMsg(port2)) { msgseen = 1 ; } if (! msgseen) Wait(mask1 | mask2 | mask3) ; } might work a little better. Remember, GetMsg() is *very* quick if no messages are on the port. And your mousemove's won't completely lock out your serial I/O. (You can change any one of the above `if (msg...' to while's, for things that *need* processing with a fair minimal latency.) > -Matt Sorry for the flaming tone of this article, Matt. I just hate people cutting down other people's coding style like that.
peter@sugar.uu.net (Peter da Silva) (02/10/89)
In article <494@infohh.rmi.de>, shimoda@infohh.rmi.de (Markus Schmidt) writes: > Every time using a Wait-GetMsg-pair I thought how to do it > best way. Now I think I got it: > while (1) { > while (!(imsg= GetMsg(win->UserPort))) > Wait (1<<win->UserPort->mp_SigBit); > ... select imsg->Class and break when ended ... > > } That's really weird. Why don't you just do this: while(1) { Wait(1<<win->UserPort->mp_SigBit); while(imsg= GetMsg(win->UserPort) { ... select imsg->Class and break when ended ... } } Which generalises to multiple message ports a lot better. -- Peter "Have you hugged your wolf today" da Silva `-_-' Hackercorp. ...texbell!sugar!peter, or peter@sugar.uu.net 'U`
dillon@POSTGRES.BERKELEY.EDU (Matt Dillon) (02/12/89)
:> while (notquit) {
:> Wait(1 << win->UserPort->mp_SigBit);
:> while (mess = GetMsg(win->UserPort)) {
:> /* process message */
:> ReplyMsg(mess);
:> }
:> }
:
:Equivalent, *except* that you have a wait at the top that's not guarded
:by a `while (GetMsg())' loop. Fine, perhaps, depending on how you enter
:the loop and what operations you allow inside the loop (what those
:subroutines might do to the signals, you know) but conservative programming
:techniques never hurt anyone.
P.S. I suppose in my old age (@ 22) I have turned to making my
code aesthetically pleasing (in a logical sense) and intuitive. Of course,
what one sees of one's own code is never what one sees of another's code or
another sees of one's own code.
The initial condition (Wait before check) is valid
because you have not ready anything from the port yet, and a re-entry
condition would also be valid because you have read all pending messages
(and the next one that comes in will set the signal).
I agree with you that, if one were using the IDCMP port for other
purposes one might have to be careful... maybe if you wanted to use it as
a sink for IO requests to (which is not unreasonable). In that case I
would put the Wait() at the bottom, but it would have to be like this:
while (notquit) {
while (mess = GetMsg(win->UserPort)) {
/* process message */
..
case CLOSEWINDOW:
notquit = 0;
break;
...
ReplyMsg(mess);
}
if (notquit)
Wait(1 << win->UserPort->mp_SigBit);
}
Otherwise the user would have to hit the close gadget twice.
: Re my multiple-IDCMP example
:Not very fair; mask1 can dominate, causing problems later. Something
:like
:
:int msgseen ;
:while (notquit) {
: msgseen = 0 ;
: if (msg = GetMsg(port1)) {
: msgseen = 1 ;
: }
: if (msg = GetMsg(port2)) {
: msgseen = 1 ;
: }
True, but only in rare circumstances. In most cases problems due to
temporary domination will not materialize.
What you outline as a solution would take quite a bit more overhead,
but a few small changes would accomplish the same thing with the same overhead
as the initial case. Unfortunetly, the codes looks a bit more messy:
mask = 0;
while (notquit) {
while (mask) {
if (mask & mask1) {
if (msg = GetMsg(port1)) {
...
} else {
mask &= ~mask1;
}
}
if (mask & mask2) {
...
}
}
if (notquit)
mask = Wait(mask1 | mask2 | mask3);
}
The overhead for an AND operation is much less than that for a
GetMsg() operation, even if there is nothing to get.
:might work a little better. Remember, GetMsg() is *very* quick if no
:messages are on the port. And your mousemove's won't completely lock
:out your serial I/O. (You can change any one of the above `if (msg...'
:to while's, for things that *need* processing with a fair minimal
:latency.)
As far as MOUSEMOVEs go, I have learned a valuable lesson:
always get ALL MOUSEMOVE messages and just execute/utilize the
last one. You can get fancy and execute/utilize whenever you come upon
a button change as well.
It would be nice if one could adjust the maximum mousemove message
rate in 1.4
:> -Matt
:
:Sorry for the flaming tone of this article, Matt. I just hate people
:cutting down other people's coding style like that.
I apologize, but it looked horrible! Never mind that it worked....
-Mattjoe@dayton.UUCP (Joseph P. Larson) (02/13/89)
Matt Dillon responded to someone's code fragment listed below. He didn't like it... This posting is my response to Tomas' response to Matt's response. In some article, rokicki@polya.Stanford.EDU (Tomas G. Rokicki) writes: >>:Now I think I got it: >>:while (1) { >>: while (!(imsg= GetMsg(win->UserPort))) >>: Wait (1<<win->UserPort->mp_SigBit); >>: select imsg->Class and break when ended >>:} > >This is *perfectly* fine. while(1)/break is a general looping >construct. while(notquit) is fine, too. > >> Yes, that's probably the worse way to do it. > >.... Secondly, his code *works*, and the >coding style is one I happen to agree with. Matter of taste, Matt. > ..... >Sorry for the flaming tone of this article, Matt. I just hate people >cutting down other people's coding style like that. I think what happened is that Matt's response might have been a little fast. I don't want to put words in Matt's mouth, but I almost posted the same message until I looked at the code a little better. The second time around I noticed the "!" on the "while" line. It makes for a very big difference in how you read the code! Personally, I don't like using a "!" in this case for two reasons: 1. It can lead to the problem I just described. "!" just isn't as noticable as one might like to indicate "I'm going to do this if this DOESN'T happen." 2. GetMsg() returns a pointer. "!" is a logic operator. Just because NULL happens to be zero doesn't mean that "!" is the correct way of checking for it. Good coding practice (as outlined by people with more letters after their name than I have) suggest you check pointers against NULL. If nothing else, it help assures better portability. (There are machines where "!" won't work. As of 2 years ago, Sperry's 1100 version of C required 1 36-bit word for the address and another byte for an offset in the 36-bit word that a pointer points to. "!" on pointers wouldn't work....) However, on the Amiga, the above code fragment will work (assuming you're only grabbing messages from one UserPort). -J -- When you fall on your head do you land on your feet? UUCP: rutgers!dayton!joe (Feed my Dayton Hudson Department Store Company ATT : (612) 375-3537 picture Joe Larson/MIS 1060 (standard disclaimer...) collection) 700 on the Mall Mpls, Mn. 55402
dillon@POSTGRES.BERKELEY.EDU (Matt Dillon) (02/14/89)
:I don't want to put words in Matt's mouth, but I almost posted the same :message until I looked at the code a little better. The second time around :I noticed the "!" on the "while" line. It makes for a very big difference :in how you read the code! Personally, I don't like using a "!" in this :case for two reasons: Nah, it wasn't fast. I never said it didn't work .. in fact, I believe I said that sure, it did work. But I had to twist my mind to read the code! -Matt
janhen@wn2.sci.kun.nl (Jan Hendrikx) (02/14/89)
In article <6402@dayton.UUCP>, joe@dayton.UUCP (Joseph P. Larson) writes: > >>: while (!(imsg= GetMsg(win->UserPort))) > > 2. GetMsg() returns a pointer. "!" is a logic operator. Just because NULL > happens to be zero doesn't mean that "!" is the correct way of checking > for it. Good coding practice (as outlined by people with more letters > after their name than I have) suggest you check pointers against NULL. > If nothing else, it help assures better portability. (There are machines > where "!" won't work. As of 2 years ago, Sperry's 1100 version of C > required 1 36-bit word for the address and another byte for an offset > in the 36-bit word that a pointer points to. "!" on pointers wouldn't > work....) Sorry, but you are fortunately not right. (!expr) means the same as (expr != 0), which is THE test to test for a NULL pointer. Even if NULL pointers are internally not represented by all-bits-zero, the compiler must recognize a comparison between a pointer and the constant 0, and emit approprate code in case that that compiler's internal representation of a NULL pointer is not the same as 0. See relatively recent articles on comp.lang.c. -Olaf Seibert.
brianr@tekig5.PEN.TEK.COM (Brian Rhodefer) (02/14/89)
I have obviously not yet attained the Zen of the Amiga, because the construct under discussion, >:while (1) { >: while (!(imsg= GetMsg(win->UserPort))) >: Wait (1<<win->UserPort->mp_SigBit); >: select imsg->Class and break when ended >:} doesn't seem "*perfectly fine*" to me at all, notwithstanding Tomas R's endorsement. Presuming that the fragment was quoted from the original posting without distortion, it sure looks to me that for the "select imsg->Class and break" portion of the code, the "imsg" pointer is NULL! Still unenlightened, Brian Rhodefer
janhen@wn2.sci.kun.nl (Jan Hendrikx) (02/14/89)
In article <331@wn2.sci.kun.nl>, janhen@wn2.sci.kun.nl I wrote: > (!expr) means the same as (expr != 0), which is THE test to test for a Of course I meant !(expr != 0). > -Olaf Seibert.
urs@Spengat.oz (Urs Ruegger) (02/18/89)
In article <494@infohh.rmi.de>, shimoda@infohh.rmi.de (Markus Schmidt) writes: > Hi! > > Every time using a Wait-GetMsg-pair I thought how to do it > best way. Now I think I got it: > > while (1) { > while (!(imsg= GetMsg(win->UserPort))) > Wait (1<<win->UserPort->mp_SigBit); > ... select imsg->Class and break when ended ... > > } Why not use : while (WaitPort(win->UserPort) { msg = GetMsg(win->UserPort); ...... ReplyMsg(msg); if (xyz) break; } WaitPort() will return as long as there are _any_ messages in your port. Only if the port is empty will it wait for the ports SigBit. Of course this will only wait for one single port, but so does your example.
dillon@POSTGRES.BERKELEY.EDU (Matt Dillon) (02/19/89)
:Why not use :
:
:while (WaitPort(win->UserPort) {
: msg = GetMsg(win->UserPort);
: ......
: ReplyMsg(msg);
: if (xyz) break;
:}
:
:WaitPort() will return as long as there are _any_ messages in your port.
:Only if the port is empty will it wait for the ports SigBit.
:
:Of course this will only wait for one single port, but so does your example.
While this will work, it doesn't make much sense. WaitPort()
ALWAYS returns the first valid message on the port. If the port is empty,
WaitPort() blocks until somebody sends a message to the port and then
returns that.
Making it the argument of a while() implies that it is possible to
return a non-zero value at some point, which it isn't. Thus, the code
will confuse the hell out of anybody with less than optimal experience with
the system library.
-Mattdan@ivucsb.UUCP (Dan Howell) (03/05/89)
In article <331@wn2.sci.kun.nl> janhen@wn2.sci.kun.nl (Jan Hendrikx) writes: |(!expr) means the same as (expr != 0), which is THE test to test for a |NULL pointer. Even if NULL pointers are internally not represented by |all-bits-zero, the compiler must recognize a comparison between a |pointer and the constant 0, and emit approprate code in case that that |compiler's internal representation of a NULL pointer is not the same |as 0. See relatively recent articles on comp.lang.c. You mean to tell me that comp.lang.c is STILL arguing about this? I was on this group in 1985 and they were arguing about it. I haven't got comp.lang.c since September because my current feed doesn't get it. Some things never change... -- Dan Howell <ivucsb!dan@anise.acc.com> -- <...!(pyramid|ucbvax)!ucsbcsl!nessus!ivucsb!dan> -- * I think this is it! My address should be relatively permanent now. *
joe@dayton.UUCP (Joseph P. Larson) (03/07/89)
>In article <331@wn2.sci.kun.nl> janhen@wn2.sci.kun.nl (Jan Hendrikx) writes: >|(!expr) means the same as (expr != 0), which is THE test to test for a >|NULL pointer. Even if NULL pointers are internally not represented by >|all-bits-zero, the compiler must recognize a comparison between a >|pointer and the constant 0, and emit approprate code in case that that >|compiler's internal representation of a NULL pointer is not the same >|as 0. Irregardless of whether this works or not, it is better style to compare against NULL, a value that indicates a pointer to nowhere. What if address zero is a legal address? NULL might be -1 then. You don't know how some odd machine is going to work in the future. So THE test for comparing a pointer against NULL is to compare it against NULL, not to depend on what NULL's value might be. !expr or expr == 0 works, but that is simply because on most machines, NULL is going to be zero. -J -- When you fall on your head do you land on your feet? UUCP: rutgers!dayton!joe (Feed my Dayton Hudson Department Store Company ATT : (612) 375-3537 picture Joe Larson/MIS 1060 (standard disclaimer...) collection) 700 on the Mall Mpls, Mn. 55402
prm@usl.usl.edu (Patrick R. Michaud) (03/09/89)
In article <6445@dayton.UUCP> joe@dayton.UUCP (Joseph P. Larson) writes: >>In article <331@wn2.sci.kun.nl> janhen@wn2.sci.kun.nl (Jan Hendrikx) writes: >>|(!expr) means the same as (expr != 0), which is THE test to test for a >>|NULL pointer. Even if NULL pointers are internally not represented by >>|all-bits-zero, the compiler must recognize a comparison between a >>|pointer and the constant 0, and emit approprate code in case that that >>|compiler's internal representation of a NULL pointer is not the same >>|as 0. Absolutely. >Irregardless of whether this works or not, it is better style to compare >against NULL, a value that indicates a pointer to nowhere. This is, of course, simply a matter of style. >What if address zero is a legal address? In 'C', zero is _never_ a legal address. Refer to section 5.4 (page 97 in my book) of "The C Programming Language" by Kernighan and Ritchie: "C guarantees that no pointer that validly points at data will contain zero, so a return value of zero can be used to signify an abnormal event.... We write NULL instead of zero, however, to indicate more clearly that this is a special value for a pointer." >NULL might be -1 then. You don't >know how some odd machine is going to work in the future. >So THE test for comparing a pointer against NULL is to compare it against >NULL, not to depend on what NULL's value might be. !expr or expr == 0 >works, but that is simply because on most machines, NULL is going to be >zero. Again, see above. If C is properly implemented, then the value of NULL almost has to be zero; or, at the very least, the compiler MUST be able to properly test pointer comparisons against zero. Personally, I really do prefer !expr over expr==0; it seems to say "if expr is not a valid pointer, then..." But again, this is probably only a matter of personal taste. >-- > When you fall on your head do you land on your feet? >UUCP: rutgers!dayton!joe (Feed my Dayton Hudson Department Store Company >ATT : (612) 375-3537 picture Joe Larson/MIS 1060 >(standard disclaimer...) collection) 700 on the Mall Mpls, Mn. 55402 //////////////////////////////////////////////////////////////////////// // Patrick R. Michaud UUCP: ...!uunet!dalsqnt!usl!prm // USL/NASA Project Leader ...!cs.utexas.edu!usl!prm // Univ. of Southwestern Louisiana INET: prm@usl.usl.edu // - "Programmer at Large" ////////////////////////////////////////////////////////////////////////